remove: remove obsolete nested roles user and admin#230
Conversation
Conventional Commits Report
🚀 Conventional commits found. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #230 +/- ##
==========================================
+ Coverage 23.52% 23.63% +0.10%
==========================================
Files 90 90
Lines 4565 4544 -21
==========================================
Hits 1074 1074
+ Misses 3414 3393 -21
Partials 77 77
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR removes the legacy nested IAM roles user and admin from the notification service’s authorization checks and updates controller route guards and tests to rely on JWT-based auth parsing (closer to production behavior) instead of the old test-only mock middleware.
Changes:
- Removed legacy
iam.User/iam.Adminroles (and corresponding authorization usage) from controllers. - Removed the old Gin-context “mock auth” middleware from test helpers and shifted tests to JWT-based auth via
integrationTests.NewTestJwtParser(). - Updated integration/unit tests to use the remaining roles (
iam.OsiUser,iam.OsiAdmin,iam.NotificationAdmin,iam.Notification) consistent with the updated authorization rules.
Reviewed changes
Copilot reviewed 34 out of 34 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| pkg/web/testhelper/helper.go | Removes obsolete mock-auth middleware helpers used by tests. |
| pkg/web/teamscontroller/useCases/updateTeamsChannel_test.go | Updates Teams channel update tests to use NotificationAdmin JWT role. |
| pkg/web/teamscontroller/useCases/listTeamsChannels_test.go | Updates Teams channel list tests to use NotificationAdmin JWT role. |
| pkg/web/teamscontroller/useCases/deleteTeamsChannel_test.go | Switches Teams delete tests to new JWT parser signature and NotificationAdmin role. |
| pkg/web/teamscontroller/useCases/createTeamsChannel_test.go | Updates Teams create tests to use NotificationAdmin JWT role. |
| pkg/web/teamscontroller/useCases/checkTeamsChannel_test.go | Switches Teams check tests to new JWT parser signature and NotificationAdmin role. |
| pkg/web/teamscontroller/teamsController.go | Removes iam.Admin from Teams controller authorization roles. |
| pkg/web/teamscontroller/teamsController_test.go | Updates Teams permission matrix and JWT parser usage to match new roles. |
| pkg/web/rulecontroller/usecases/updateRule_test.go | Updates rule update tests to use NotificationAdmin JWT role. |
| pkg/web/rulecontroller/usecases/listRules_test.go | Updates rule list tests to use NotificationAdmin JWT role. |
| pkg/web/rulecontroller/usecases/getRule_test.go | Updates rule get tests to use NotificationAdmin JWT role. |
| pkg/web/rulecontroller/usecases/deleteRule_test.go | Updates rule delete tests to use NotificationAdmin JWT role. |
| pkg/web/rulecontroller/usecases/createRule_test.go | Switches rule create tests to new JWT parser signature and NotificationAdmin role. |
| pkg/web/rulecontroller/ruleController.go | Removes iam.Admin from Rule controller authorization roles. |
| pkg/web/rulecontroller/ruleController_test.go | Updates Rule permission matrix and JWT parser usage to match new roles. |
| pkg/web/origincontroller/originController_test.go | Replaces mock middleware with real JWT middleware and uses Notification JWT role. |
| pkg/web/notificationcontroller/useCases/forward_notification_test.go | Updates forwarding integration test setup to new JWT parser signature and roles. |
| pkg/web/notificationcontroller/notificationController.go | Removes iam.User from list endpoints’ allowed roles. |
| pkg/web/notificationcontroller/notificationController_test.go | Updates JWT parser usage and role expectations in permissions + list tests. |
| pkg/web/mattermostcontroller/useCases/updateMattermostChannel_test.go | Updates Mattermost channel update tests to use NotificationAdmin JWT role. |
| pkg/web/mattermostcontroller/useCases/listMattermostChannels_test.go | Updates Mattermost channel list tests to use NotificationAdmin JWT role. |
| pkg/web/mattermostcontroller/useCases/deleteMattermostChannel_test.go | Switches Mattermost delete tests to new JWT parser signature and NotificationAdmin role. |
| pkg/web/mattermostcontroller/useCases/createMattermostChannel_test.go | Updates Mattermost create tests to use NotificationAdmin JWT role. |
| pkg/web/mattermostcontroller/useCases/checkMattermostChannel_test.go | Switches Mattermost check tests to new JWT parser signature and NotificationAdmin role. |
| pkg/web/mattermostcontroller/mattermostController.go | Removes iam.Admin from Mattermost controller authorization roles. |
| pkg/web/mattermostcontroller/mattermostController_test.go | Updates Mattermost permission matrix and JWT parser usage to match new roles. |
| pkg/web/mailcontroller/negative_usecases_test.go | Adds JWT auth to mail negative integration tests using NotificationAdmin role. |
| pkg/web/mailcontroller/mailController.go | Removes iam.Admin from Mail controller authorization roles. |
| pkg/web/mailcontroller/mailController_test.go | Replaces mock middleware with JWT auth middleware and adds JWT to requests. |
| pkg/web/mailcontroller/mailController_integration_test.go | Adds JWT auth middleware + JWT usage to mail CRUD integration tests. |
| pkg/web/mailcontroller/checkMailServer.go | Removes iam.Admin from mail server check endpoint authorization roles. |
| pkg/web/mailcontroller/checkMailServer_test.go | Switches to new JWT parser signature and updates permission matrix. |
| pkg/web/integrationTests/testJwtParser.go | Changes NewTestJwtParser to no longer require *testing.T. |
| pkg/web/iam/roles.go | Removes legacy User and Admin role constants. |
Comments suppressed due to low confidence (1)
pkg/web/iam/roles.go:13
- The comment on NotificationAdmin still references the removed Admin constant ("Same as Admin"), which is now misleading. Please update the comment to reflect the current meaning without referring to a non-existent role.
Notification = "opensight_notification_role" // Only a service user
NotificationAdmin = "notification.admin" // Same as Admin
)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
they are legacty roles which are no longer to be used for authorization
02d5a4d to
d95802a
Compare
What
remove: remove obsolete nested roles
userandadminAlso removed old middleware approach which just sets the token and rely on
integrationTests.NewTestJwtParserandintegrationTests.NewTestJwtParser(). This is closer to the real production setup and verifies that the used nested role in the token is among the required roles by the respective endpoint.Why
They are legacy roles which are no longer to be used for authorization
References
ARTOSI-535