Add support for multiple projects #28
Conversation
rickary
force-pushed
the
26-support-multiple-projects
branch
from
a5d556e to
e811f7a
Compare
flagsmith-jira-app/src/frontend/components/IssueFeatureTables.tsx
Outdated
Show resolved
Hide resolved
flagsmith-jira-app/src/frontend/components/IssueFeatureTables.tsx
Outdated
Show resolved
Hide resolved
flagsmith-jira-app/src/frontend/components/IssueFeatureTables.tsx
Outdated
Show resolved
Hide resolved
flagsmith-jira-app/src/frontend/components/IssueFeatureTables.tsx
Outdated
Show resolved
Hide resolved
rickary
force-pushed
the
26-support-multiple-projects
branch
2 times, most recently
from
cacdeda to
11a0f72
Compare
wip rm some logs working loading indicator works correctly project setting self cr cr rebase project name
rickary
force-pushed
the
26-support-multiple-projects
branch
from
11a0f72 to
8f9fc26
Compare
|
|
||
| const allFeatures: Feature[] = []; | ||
| const organisationId = await readOrganisationId(); | ||
| const projects = await readProjects({ organisationId }); |
There was a problem hiding this comment.
I'm a bit concerned here by the amount of requests we are doing.
Basically here we map over the environments
return await Promise.all(
environments.map((environment) =>
readFeatures({ projectIds, environmentId: String(environment.id) }),
And making this request for each of them to later do a find.
The projects shouldn't change from one request to another so could we find a way to request once and re-use the result directly ? Maybe having the projects as an argument directly ?
|
|
||
| const allFeatures: Feature[] = []; | ||
| const organisationId = await readOrganisationId(); | ||
| const projects = await readProjects({ organisationId }); |
There was a problem hiding this comment.
There is a trust issue here. The client-side is untrustworthy, so a Jira user with the correct IDs can potentially retrieve data for projects that are available to the API key even if they are not connected to the Flagsmith organisation that the Jira instance is associated with.
This is why the all the backend Flagsmith API calls retrieve the organisation ID from the Jira instance and then the projects for that organisation from the Flagsmith API and then use this to filter any project IDs provided by the client. The trade-off is that makes more API calls than is ideal.
Having the client-side provide the project list bypasses this check, so I don't think this is a good idea.
The correct fix is to use organisation-scoped (RBAC) API keys, which I think is the next scheduled work item? When an RBAC API key is in use, there is no need for the backend to retrieve projects for filtering as any API call to a project ID that isn't permitted will cause an auth error.
This particular backend function also uses the project list to annotate the features with the project name as a convenience but this could be done on the client-side with the backend annotating with the project ID instead.
If API loading is a genuine issue in the interim then adding some jitter to the calls in the backend could help?
There was a problem hiding this comment.
Ok thanks for the detailed answer @riceyrice 🙏 .
I was unaware that RBAC was an incoming item.
In that case it make sense to keep the security over efficiency trade-off for now. The traffic is acceptable at the moment. But can we add a comment to keep under the radar the improvement and properly fix it once RBAC is tackled
There was a problem hiding this comment.
@Zaimwa9 I've reverted the changes relating to the making multiple requests and added a comment linking to the github issue.
rickary
force-pushed
the
26-support-multiple-projects
branch
from
383f596 to
bae9697
Compare
Note: branched off #29 as how that works directly affects this feature