-
Notifications
You must be signed in to change notification settings - Fork 678
SMQ-2627 - Align PATs with new architecture #3295
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3295 +/- ##
==========================================
- Coverage 76.52% 73.68% -2.85%
==========================================
Files 15 179 +164
Lines 1423 19701 +18278
==========================================
+ Hits 1089 14516 +13427
- Misses 246 4330 +4084
- Partials 88 855 +767 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
7aa84c3 to
8976860
Compare
| } | ||
|
|
||
| if strings.HasPrefix(token, patPrefix) { | ||
| tokenType := authn.TokenType(res.GetTokenType()) |
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.
This creates possible misconfiguration issue. I.e. what if token prefix here and tokenType returned from the service are not the same? While we are not able to validate token - we are able to parse it locally (even if only for prefix) and we can do this verification to prevent mismatch.
channels/middleware/authorization.go
Outdated
| } else { | ||
| req.Operation = auth.ListOp | ||
| } | ||
| req.Operation = auth.ChannelDisconnectClientOp |
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.
Let's stay consistent in naming, compare this to ClientConnectToChannelOp.
| UnshareOp | ||
| PublishOp | ||
| SubscribeOp | ||
| ClientCreateOp Operation = iota + 100 |
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.
Is it possible to reuse the existing service Operations ?
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.
we will have cyclic imports
d2f93f8 to
9632e74
Compare
Signed-off-by: nyagamunene <[email protected]>
Signed-off-by: nyagamunene <[email protected]>
Signed-off-by: nyagamunene <[email protected]>
Signed-off-by: nyagamunene <[email protected]>
Signed-off-by: nyagamunene <[email protected]>
Signed-off-by: nyagamunene <[email protected]>
Signed-off-by: nyagamunene <[email protected]>
Signed-off-by: nyagamunene <[email protected]>
Signed-off-by: nyagamunene <[email protected]>
Signed-off-by: nyagamunene <[email protected]>
81bc751 to
30bd274
Compare
What type of PR is this?
What does this do?
Which issue(s) does this PR fix/relate to?
Have you included tests for your changes?
Did you document any new/modified feature?
Notes