feat: read logs and update cors maintenance root-role permissions #8996
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.OpenSSF Scorecard
Scanned Files |
Tymek
force-pushed
the
feat/more-maintenance-permissions
branch
from
343a857 to
f8a62ef
Compare
|
@Tymek, core features have been modified in this pull request. Please review carefully! |
github-actions
bot
requested review from
chriswk,
FredrikOseberg,
gastonfournier and
ivarconr
| async setCors( | ||
| req: IAuthRequest<void, void, SetCorsSchema>, | ||
| res: Response<string>, | ||
| ): Promise<void> { | ||
| const granularAdminPermissions = this.flagResolver.isEnabled( | ||
| 'granularAdminPermissions', | ||
| ); | ||
|
|
||
| if (!granularAdminPermissions) { | ||
| throw new NotFoundError(); | ||
| } | ||
|
|
||
| if (req.body.frontendApiOrigins) { | ||
| await this.frontendApiService.setFrontendCorsSettings( | ||
| req.body.frontendApiOrigins, | ||
| req.audit, | ||
| ); | ||
| res.sendStatus(204); | ||
| return; | ||
| } | ||
|
|
||
| throw new NotFoundError(); | ||
| } | ||
| } |
There was a problem hiding this comment.
Previous "setUiConfig" is a generic name, but it only sets CORS. For easy transition into endpoint with proper permissions it's easier to create new one, and avoid potential errors or confusion about permissions in the future.
There was a problem hiding this comment.
Alternatively we could use a PATCh on the uiConfig, but we're not using PATCH that frequently... but arguably CORS is a configuration on itself, maybe it's fine not having it as part of uiConfig
|
Hi @Tymek what's the reasoning behind this PR? It's hard for me to understand why this change (i.e. context) |
|
@gastonfournier It's to enable creating custom root-roles, that have access to only some responsibilities of Admin user. Ticket: https://linear.app/unleash/issue/CJUX-278/[roles]-custom-root-roles-missing-admin-responsibilities |
| @@ -99,6 +100,7 @@ class ConfigController extends Controller { | |||
| ], | |||
| }); | |||
|
|
|||
| // TODO: deprecate when removing `granularAdminPermissions` flag | |||
There was a problem hiding this comment.
This would be a breaking change so we have to delay removing it. Meanwhile I believe you can flag the route as deprecated (for OpenAPI doc)
There was a problem hiding this comment.
Yes, that's the process I'll follow.
| async setCors( | ||
| req: IAuthRequest<void, void, SetCorsSchema>, | ||
| res: Response<string>, | ||
| ): Promise<void> { | ||
| const granularAdminPermissions = this.flagResolver.isEnabled( | ||
| 'granularAdminPermissions', | ||
| ); | ||
|
|
||
| if (!granularAdminPermissions) { | ||
| throw new NotFoundError(); | ||
| } | ||
|
|
||
| if (req.body.frontendApiOrigins) { | ||
| await this.frontendApiService.setFrontendCorsSettings( | ||
| req.body.frontendApiOrigins, | ||
| req.audit, | ||
| ); | ||
| res.sendStatus(204); | ||
| return; | ||
| } | ||
|
|
||
| throw new NotFoundError(); | ||
| } | ||
| } |
There was a problem hiding this comment.
Alternatively we could use a PATCh on the uiConfig, but we're not using PATCH that frequently... but arguably CORS is a configuration on itself, maybe it's fine not having it as part of uiConfig
Co-authored-by: Gastón Fournier <[email protected]>
yes, it's also easier to find if it's 1:1 with action on a dedicated page in admin panel |
About the changes
Additional granular permissions related to instance-level access.