This repository has been archived by the owner on Oct 9, 2023. It is now read-only.
Introduce pre redirect hook plugin during auth callback #601
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pmahindrakar-oss
requested review from
katrogan,
wild-endeavor,
EngHabu,
kumare3,
hamersaw and
eapolinario
as code owners
pmahindrakar-oss
changed the title
Codecov Report
@@ Coverage Diff @@
## master #601 +/- ##
==========================================
+ Coverage 58.66% 60.27% +1.61%
==========================================
Files 171 171
Lines 16464 13456 -3008
==========================================
- Hits 9659 8111 -1548
+ Misses 5954 4493 -1461
- Partials 851 852 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
|
katrogan
reviewed
Aug 10, 2023
| logger.Infof(ctx, "preRedirect hook is set") | ||
| if err := preRedirectHook(ctx, authCtx, request, writer); err != nil { | ||
| logger.Errorf(ctx, "failed the preRedirect hook due %v with status code %v", err.Message, err.Code) | ||
| if http.StatusText(err.Code) != "" { |
There was a problem hiding this comment.
should we also check that it isn't status ok? https://go.dev/src/net/http/status.go
There was a problem hiding this comment.
So i am relying on the user code which set the hook to set the correct http status code .
If they choose to set OK, then thats what we will set on the header.
Signed-off-by: pmahindrakar-oss <[email protected]>
Signed-off-by: pmahindrakar-oss <[email protected]>
Signed-off-by: pmahindrakar-oss <[email protected]>
Signed-off-by: pmahindrakar-oss <[email protected]>
Signed-off-by: pmahindrakar-oss <[email protected]>
Signed-off-by: pmahindrakar-oss <[email protected]>
Signed-off-by: pmahindrakar-oss <[email protected]>
Signed-off-by: pmahindrakar-oss <[email protected]>
Signed-off-by: pmahindrakar-oss <[email protected]>
Signed-off-by: pmahindrakar-oss <[email protected]>
pmahindrakar-oss
force-pushed
the
introduce-callback-plugin
branch
from
33a2369
to
c7b92c7
Compare
katrogan
approved these changes
Aug 10, 2023
eapolinario
pushed a commit
that referenced
this pull request
Sep 6, 2023
* Adding a predredirect hook plugin Signed-off-by: pmahindrakar-oss <[email protected]> * Add test logs Signed-off-by: pmahindrakar-oss <[email protected]> * test logs Signed-off-by: pmahindrakar-oss <[email protected]> * fix Signed-off-by: pmahindrakar-oss <[email protected]> * Reading identity token for getting subject Signed-off-by: pmahindrakar-oss <[email protected]> * reverting Signed-off-by: pmahindrakar-oss <[email protected]> * Adding PreRedirectHookError Signed-off-by: pmahindrakar-oss <[email protected]> * Add some more tests Signed-off-by: pmahindrakar-oss <[email protected]> * lint fixes Signed-off-by: pmahindrakar-oss <[email protected]> * removed log line Signed-off-by: pmahindrakar-oss <[email protected]> --------- Signed-off-by: pmahindrakar-oss <[email protected]>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
TL;DR
Introduces a preRedirectHook plugin handler to allow custom code to be able to be plugged in before the successful auth call back redirect happens
Type
Are all requirements met?
Complete description
PreRedirectHookFunc Interface is used for running custom code before the redirect happens during a successful auth flow.
The API allows you to access the authenticationContext, request and response
This might be useful in cases where the auth flow allows the user to login since the IDP has been configured
for eg: to allow all users from a particular domain to login but you want to restrict access to only a particular set of user ids. eg : [email protected] are allowed to login but user [email protected], [email protected] should only be allowed
The current Plugin hook will allow you to add such a custom code.
PreRedirectHookError is the error interface which allows the user to set correct http status code and Message to be set in case the function returns an error without which the current usage in GetCallbackHandler will set this to InternalServerError
Tracking Issue
fixes flyteorg/flyte#3940
Follow-up issue
NA