Skip to content
This repository has been archived by the owner on Jan 27, 2021. It is now read-only.

Use roleIDs from context instead of loading them again #69

Merged
merged 6 commits into from
Sep 2, 2020

Conversation

kulmann
Copy link
Member

@kulmann kulmann commented Sep 1, 2020

The roleIDs are coming in as json string from the metadata context and can be extracted
using a helper function from the roles middleware. Instead of re-loading
those roleIDs again on every permission related request, we now just use
the roleIDs from the context. This is also more future-proof for when we
decide to move away the role assignments from the settings service into
the accounts service.

The roleIDs are coming in as json string from the metadata context and can be extracted
using a helper function from the roles middleware. Instead of re-loading
those roleIDs again on every permission related request, we now just use
the roleIDs from the context. This is also more future-proof for when we
decide to move away the role assignments from the settings service into
the accounts service.
@kulmann kulmann requested review from C0rby and IljaN September 2, 2020 06:58
@@ -267,7 +264,7 @@ func (g Service) ListRoles(c context.Context, req *proto.ListBundlesRequest, res
if err != nil {
return merrors.NotFound(g.id, "%s", err)
}
// TODO: only allow to list roles when user has account management permissions
// TODO: only allow to list roles when user has account/role/... management permissions
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

@PVince81 PVince81 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@PVince81 PVince81 merged commit bc0ac44 into master Sep 2, 2020
@delete-merged-branch delete-merged-branch bot deleted the use-role-ids-from-context branch September 2, 2020 09:30
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants