- Notifications
You must be signed in to change notification settings - Fork669
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
SMQ-2632 - Separate PAT logic in middleware#2636
base:main
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@## main #2636 +/- ##==========================================- Coverage 42.32% 34.40% -7.93%========================================== Files 350 243 -107 Lines 47867 40108 -7759 ==========================================- Hits 20259 13798 -6461+ Misses 25402 25219 -183+ Partials 2206 1091 -1115 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
@arvindh123 Please review this one. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
@nyagamunene Please resolve conflicts.
8eb9581 to4ae49e8Compare3d4f6e6 to816a0e3CompareThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
We can merge this PR after merging of#2680
acf54d5 toe1f6026CompareThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
@nyagamunene What's the status with this one? Please rebase it.
@dborovcanin This is ready I have rebased it. |
clients/middleware/authorization.go Outdated
| if err := am.checkSuperAdmin(ctx, session.UserID); err != nil { | ||
| return clients.ClientsPage{}, err |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Please revert the logic here, superadmins and non superadmins can list clients, just set theSuperAdmin boolean
cmd/channels/main.go Outdated
| @@ -195,6 +196,15 @@ func main() { | |||
| defer authzClient.Close() | |||
| logger.Info("AuthZ successfully connected to auth gRPC server " + authzClient.Secure()) | |||
| pat, patHandler, err := authsvcPat.NewAuthorization(ctx, grpcCfg) | |||
| if err != nil { | |||
| logger.Error("failed to create authz " + err.Error()) | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
could you make this more decriptive to pats authz
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
Signed-off-by: nyagamunene <stevenyaga2014@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
What type of PR is this?
This is a refactor because it seperates PATs logic from Authz middleware
What does this do?
It seperates PATs logic from Authz middleware for users and clients
Which issue(s) does this PR fix/relate to?
Have you included tests for your changes?
Yes
Did you document any new/modified feature?
No
Notes