Skip to content
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

Ability to define NoRedirect=true per URL path #541

Open
p53 opened this issue Jan 13, 2025 · 6 comments · Fixed by #542
Open

Ability to define NoRedirect=true per URL path #541

p53 opened this issue Jan 13, 2025 · 6 comments · Fixed by #542
Assignees
Milestone

Comments

@p53
Copy link

p53 commented Jan 13, 2025

Title

Ability to define NoRedirect=true per URL path, requested by user in #532

Summary

Right now there is NoRedirect option, which is global and defines if gatekeeper uses AuthorizationCode flow for getting token or API Bearer token to authenticate incoming request. Problem is that if you want to use both e.g. AuthorizationCode flow for some part of your endpoints and API flow for different set of endpoints you have to use two gatekeepers.

Why?

With this feature user will be able to define no-redirect=true per resource, so user will be able to use one gatekeeper to protect both e.g. user facing pages and api endpoints

How

we will add new option in resources, no-redirect, which is only considered when no-redirect=true

Acceptance criteria

backward compatibility, ability to make no-redirect=true per path

Additional Information

@p53 p53 self-assigned this Jan 13, 2025
@p53 p53 added this to Gatekeeper Jan 13, 2025
@p53 p53 added this to the 3.1.0 milestone Jan 13, 2025
@p53 p53 linked a pull request Jan 13, 2025 that will close this issue
2 tasks
@p53 p53 closed this as completed in #542 Jan 13, 2025
@github-project-automation github-project-automation bot moved this from To do to Done in Gatekeeper Jan 13, 2025
@p53 p53 reopened this Jan 13, 2025
@github-project-automation github-project-automation bot moved this from Done to In progress in Gatekeeper Jan 13, 2025
@p53
Copy link
Author

p53 commented Jan 13, 2025

@jackhg96 @colla @stemyers @oyelekci @aryakumargeorge @hitesh-ladha you can try 3.1.0-rc1 release, there is also docker image

Quick docu, you can set no-redirect=true per resource:

no-redirects: false
resources:
- uri: /admin/test
  # the methods on this URL that should be protected, uri is required when defining resource
  method: GET
  no-redirect: true

IMPORTANT!!: the other way it doesn't work (because of technical reasons to keep backward compatibility):

# this won't work
no-redirects: true
resources:
- uri: /admin/test
  # the methods on this URL that should be protected, uri is required when defining resource
  method: GET
  no-redirect: false

@aryakumargeorge
Copy link

@p53 thank you for the update and the quick turn around with a solution for this issue.
With testing I have hit a small problem, where the new resource structure field does not have a json definition. I fixed locally and tested the solution successfully. Could you please fix this? thanks.
in the resource.go file
//existing code
// NoRedirect overrides global no-redirect setting
NoRedirect bool

//fixed code
// NoRedirect overrides global no-redirect setting
NoRedirect bool json:"no-redirect" yaml:"no-redirect"

@p53
Copy link
Author

p53 commented Jan 14, 2025

@aryakumargeorge yup, thanks for checking!

@p53
Copy link
Author

p53 commented Jan 14, 2025

@aryakumargeorge created 3.1.0-rc2 with fix

@aryakumargeorge
Copy link

@p53 new image working as expected, thank you.
Are you going to create a new image with minor version update, or should I stick to 3.1.0-rc2?

@p53
Copy link
Author

p53 commented Jan 15, 2025

@aryakumargeorge i would like to add some additional things to this release so for the time being (week or so) stick to 3.1.0-rc2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: In progress
Development

Successfully merging a pull request may close this issue.

2 participants