-
Notifications
You must be signed in to change notification settings - Fork 672
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
SMQ - 2724 - Add Auth Callout #2731
base: main
Are you sure you want to change the base?
Conversation
7119d7b
to
910f660
Compare
auth/callback.go
Outdated
if len(c.urls) == 0 { | ||
return nil | ||
} |
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.
Move this to the beginning of the function. Also, let's use a single URL at a time, with other URLs being a fallback, something like https://www.rabbitmq.com/docs/access-control#combined-backends.
26e4b56
to
c24229d
Compare
TraceRatio float64 `env:"SMQ_JAEGER_TRACE_RATIO" envDefault:"1.0"` | ||
ESURL string `env:"SMQ_ES_URL" envDefault:"nats://localhost:4222"` | ||
AuthCalloutURLs []string `env:"SMQ_AUTH_CALLOUT_URLS" envDefault:"" envSeparator:","` | ||
AuthCalloutMethod string `env:"SMQ_AUTH_CALLOUT_METHOD" envDefault:"POST"` |
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 can try them iteratively on failure POST, then GET
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.
I prefer not to add these mechanisms since we already have the flexibility to specify the HTTP method type in the environment configuration. This allows us to manage different request methods effectively without adding extra logic for retries. If the user mistakenly provides POST instead of GET, it would be their responsibility to correct this in the configuration.
@@ -146,6 +149,9 @@ SMQ_JAEGER_URL=http://localhost:14268/api/traces \ | |||
SMQ_JAEGER_TRACE_RATIO=1.0 \ | |||
SMQ_SEND_TELEMETRY=true \ | |||
SMQ_AUTH_ADAPTER_INSTANCE_ID="" \ | |||
SMQ_AUTH_CALLOUT_URLS="" \ | |||
SMQ_AUTH_CALLOUT_METHOD="POST" \ | |||
SMQ_AUTH_CALLOUT_TLS_VERIFICATION=true \ |
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 do you think about having option for request timeout ?
cmd/auth/main.go
Outdated
svc := auth.New(keysRepo, patsRepo, hasher, idProvider, t, pEvaluator, pService, cfg.AccessDuration, cfg.RefreshDuration, cfg.InvitationDuration) | ||
httpClient := &http.Client{ | ||
Transport: &http.Transport{ | ||
TLSClientConfig: &tls.Config{ |
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.
Can we add support for custom certification for the HTTPS request ?
6d17b81
to
da7fadc
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2731 +/- ##
==========================================
- Coverage 42.03% 34.76% -7.28%
==========================================
Files 347 212 -135
Lines 47929 38165 -9764
==========================================
- Hits 20146 13267 -6879
+ Misses 25582 23821 -1761
+ Partials 2201 1077 -1124 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Rodney Osodo <[email protected]>
…nt purposes for policy enforcement Signed-off-by: Rodney Osodo <[email protected]>
2e38c4b
to
74532fb
Compare
Signed-off-by: Rodney Osodo <[email protected]>
Signed-off-by: Rodney Osodo <[email protected]>
Signed-off-by: Rodney Osodo <[email protected]>
Signed-off-by: Rodney Osodo <[email protected]>
Signed-off-by: Rodney Osodo <[email protected]>
Signed-off-by: Rodney Osodo <[email protected]>
Signed-off-by: Rodney Osodo <[email protected]>
Signed-off-by: Rodney Osodo <[email protected]>
Signed-off-by: Rodney Osodo <[email protected]>
Signed-off-by: Rodney Osodo <[email protected]>
Signed-off-by: Rodney Osodo <[email protected]>
…ests Signed-off-by: Rodney Osodo <[email protected]>
Signed-off-by: Rodney Osodo <[email protected]>
3654abf
to
d8699bb
Compare
What type of PR is this?
This is a feature pull request that introduces external authorization callouts to the Auth service. It adds new configuration options, a callback mechanism, and integrates it into the authorization flow. The changes include modifications to the service logic, configuration, and testing infrastructure.
What does this do?
Which issue(s) does this PR fix/relate to?
Have you included tests for your changes?
Yes, I have included tests for my changes.
Did you document any new/modified feature?
Yes, I have updated the documentation for the new feature.
Notes