(User Roles) Remove legacy scopes #7792
Conversation
This comment has been minimized.
This comment has been minimized.
Nil20
force-pushed
the
remove-legacy-scopes
branch
from
149a267
to
85f7810
Compare
There was a problem hiding this comment.
Barry has created this Scopes constant: https://github.com/opencrvs/opencrvs-core/blob/configurable-roles/packages/commons/src/scopes.ts#L12
Let's use this instead
Nil20
added
💅 Waiting For Review
🚧 work in progress
and removed
🚧 work in progress
💅 Waiting For Review
labels
Nil20
force-pushed
the
remove-legacy-scopes
branch
2 times, most recently
from
d745128
to
fbc2abe
Compare
Nil20
force-pushed
the
remove-legacy-scopes
branch
from
fbc2abe
to
973425a
Compare
Nil20
added
🚧 work in progress
💅 Waiting For Review
and removed
💅 Waiting For Review
🚧 work in progress
labels
There was a problem hiding this comment.
We don't really have a one to one scope mapping for the previous sysadmin role/scope. Let's just remove it's usages and not try to add any other scope as it's replacement. Or @rikukissa @jpye-finch if you have any better idea
| @@ -74,6 +74,7 @@ export const SCOPES = { | |||
| RECORD_REGISTRATION_REQUEST_REINSTATEMENT: | |||
| 'record.registration-request-reinstatement', | |||
| RECORD_REGISTRATION_REINSTATE: 'record.registration-reinstate', | |||
| RECORD_CERTIFY: 'record.certify', | |||
There was a problem hiding this comment.
@Nil20 Do we have a scope like that in notion? To me the equivalent for our previous 'certify' scope would be record.declaration-print
There was a problem hiding this comment.
@Nil20 We need to use specific user.* scopes instead of config.update:all
| SCOPES.RECORD_DECLARE_BIRTH, | ||
| SCOPES.RECORD_DECLARE_DEATH, | ||
| SCOPES.RECORD_DECLARE_MARRIAGE, |
There was a problem hiding this comment.
| SCOPES.RECORD_DECLARE_BIRTH, | |
| SCOPES.RECORD_DECLARE_DEATH, | |
| SCOPES.RECORD_DECLARE_MARRIAGE, | |
| SCOPES.RECORD_DECLARE_BIRTH, |
As this is for a birth event. We should do the same for the other endpoints too
| @@ -416,7 +416,7 @@ export default function getRoutes(): ServerRoute<ReqRefDefaults>[] { | |||
| tags: ['api'], | |||
| description: 'Sends an sms to a user with new temporary password', | |||
| auth: { | |||
| scope: [RouteScope.SYSADMIN] | |||
| scope: [SCOPES.CONFIG_UPDATE_ALL] | |||
There was a problem hiding this comment.
a more appropriate one would probably be user:edit?
| SCOPES.RECORD_REGISTER, | ||
| SCOPES.PERFORMANCE_READ, | ||
| SCOPES.RECORD_SUBMIT_FOR_APPROVAL, | ||
| SCOPES.CONFIG_UPDATE_ALL |
There was a problem hiding this comment.
We shouldn't need to keep the CONFIG_UPDATE_ALL scope here
There was a problem hiding this comment.
The search routes shouldn't be tied to sysadmin related scopes
Farajaland PR: opencrvs/opencrvs-farajaland#1142