Add support for per route authorization #2011

ibizaman · 2023-02-11T10:03:57Z

Description

Add the possibility to associate a group - and/or role for
Keycloak-OIDC - with a path prefix. This means we can have fine
grained control over which group and/or role to allow per request
path.

The following configuration:

allowed_roles = [ "user", "admin", "superadmin", "/admin|admin", "/admin|superadmin" ]

Gets mapped internally to:

path: /
  - user
  - admin
  - superadmin
path: /admin
  - admin
  - superadmin

So only those having the role admin or superadmin can access
anything under the /admin path.

Same thing when substituting "role" for "group".

A few caveats:

I chose to extend the existing allowed_groups and allowed_roles
syntax to [<path>|]<name>. This choice is completely arbitrary and
can be changed if something else makes more sense.
If we choose to keep the extended syntax, I should probably add a
group_path_separator argument to let the user choose another
separator and default it to |.
I chose to only match on path prefix. No Regexp, path suffix, method
matching, etc. Why? It was simpler to get a PoC out of the door. I'm
happy to extend this to other use cases I didn't think about, of
course.

Motivation and Context

This feature allows more fine-grained control on what path is
accessible to what users.

It allows oauth2-proxy to become an even more indispensable building
block to protect apps because we can rely on oauth2-proxy to implement
security that was only possible in-app beforehand, freeing the
developers to work on their business use case.

But more importantly, at least for my use case, we can add protection
over apps that we cannot update. For example, here I wanted to protect
a Vaultwarden instance that does not handle SSO
yet. Alos,
Vaultwarden exposes an admin UI under /admin path. I wanted to add
SSO in front of the whole Vaultwarden UI but also to protect the
/admin path with stricter role. This PR makes this possible.

How Has This Been Tested?

First, I added some unit tests.

Also, I deployed oauth2-proxy in front of a Vaultwarden service. oauth2-proxy's config included in particular:

allowed_roles = [ "user", "/admin|admin" ]

The Keycloak instance has two user:

"lowpriviledge" having a realm role of user
and "highpriviledge" having a realm role of admin which is a
composite role including the user role.

When logging with "lowpriviledge", I can access the normal Vaultwarden
UI under / path while I get a 403 if I try to access anything under
/admin. While with "highpriviledge" I can access the whole UI.

Checklist:

I purposely didn't add documentation yet as I wanted to gather
feedback first. But I'll of course add some when we settle on what
arguments to have and such.

My change requires a change to the documentation or CHANGELOG.
I have updated the documentation/CHANGELOG accordingly.
I have created a feature (non-master) branch for my PR.

github-actions · 2023-04-18T00:06:03Z

This pull request has been inactive for 60 days. If the pull request is still relevant please comment to re-activate the pull request. If no action is taken within 7 days, the pull request will be marked closed.

ibizaman · 2023-04-19T03:46:49Z

Relevant! Already 60 days… time flies.

sebwills · 2023-05-10T17:21:09Z

providers/provider_default_test.go

+			expectedAuthZ: true,
+		},
+		{
+			name:          "MoreSpecificPathMatches",


Equating "more specific" to "longer prefix match" is ambiguous and potentially misleading?

Which is "more specific" - /administration or /admin/a/b? I think most people would say the latter, because it is 'deeper'. But given a rule for /administration and a rule for /admin/a/b, your code will pick the former as being "more specific" and hence the rule that is used.

An alternative to consider would be to use the first match in the list?

(I'm not associated with the project, this is a drive-by comment!)

This makes sense indeed. Maybe we should allow globing or regex. It’s more cognitive load on the user but at the same time the contract is clear.

I can imagine some use cases might benefit from regex/glob matching. But I think it's an orthogonal question - you could still have multiple matches and need a way to pick one, i.e. first wins.

I see what you mean. Probably first wins is more intuitive.

sebwills · 2023-05-10T17:21:42Z

This feature would be super useful. What needs to be done to attract some feedback from the maintainers?

github-actions · 2023-07-12T00:07:15Z

This pull request has been inactive for 60 days. If the pull request is still relevant please comment to re-activate the pull request. If no action is taken within 7 days, the pull request will be marked closed.

ibizaman · 2023-07-15T05:31:34Z

still relevant!

ghmer · 2023-07-15T17:59:22Z

Exactly what I was looking for. Please get this merged into oauth2-proxy!

henriduflot · 2023-09-04T13:51:53Z

That is exactly what we need. We have a lot a tools which don't support authentication but we need oauth2_proxy to protected it based on users groups.

github-actions · 2023-11-13T00:06:21Z

This pull request has been inactive for 60 days. If the pull request is still relevant please comment to re-activate the pull request. If no action is taken within 7 days, the pull request will be marked closed.

ibizaman · 2023-11-13T07:17:05Z

Would love to get this merged although TBH I since switched from Keycloak to Authelia which does support this out of the box :/

igops · 2023-12-01T11:58:49Z

@ibizaman great stuff! +1 for merging into the main branch from me!

I was going to make a similar fork by myself. However, my idea was to keep a configuration untouched and manage per-route roles on the reverse-proxy like:

location /my-location {
    set $roles "admin,viewer,commentor";
    # auth_request block with some additional params ...
    # proxying logic ...
}

location = /oauth2/auth {
    # pass the roles to the oauth2-proxy somehow (in headers?) ...
    # auth logic ...
}

I haven't tried my version yet 😄

github-actions · 2024-01-31T00:06:21Z

This pull request has been inactive for 60 days. If the pull request is still relevant please comment to re-activate the pull request. If no action is taken within 7 days, the pull request will be marked closed.

haeraeus · 2024-01-31T06:46:16Z

Would love to get this merged too ❤️

github-actions · 2024-04-03T00:06:37Z

This pull request has been inactive for 60 days. If the pull request is still relevant please comment to re-activate the pull request. If no action is taken within 7 days, the pull request will be marked closed.

sebwills · 2024-04-05T18:00:35Z

@ibizaman does your 👍 on the bot's post mean you don't think this is worth pursuing any more? Personally I don't need it any more but seems like a useful feature still?

ibizaman · 2024-04-05T19:36:46Z

@sebwills there was a bit of regnisation indeed. I’m not using this project anymore but I’d be happy to get to merging it if there is some interest for it.

github-actions · 2024-06-05T00:07:04Z

This pull request has been inactive for 60 days. If the pull request is still relevant please comment to re-activate the pull request. If no action is taken within 7 days, the pull request will be marked closed.

pushc6 · 2024-06-25T03:20:49Z

I would love to see this feature added, it would be extremely useful.

github-actions · 2024-08-28T00:07:15Z

This pull request has been inactive for 60 days. If the pull request is still relevant please comment to re-activate the pull request. If no action is taken within 7 days, the pull request will be marked closed.

faresismail96 · 2024-09-08T02:21:24Z

would love to have this feature as well!

ctschubel · 2024-09-25T07:02:59Z

Coming from louketo-proxy/keycloak-proxy/keycloak-gatekeeper, this feature would help a great deal setting up oauth2-proxy for my apps!

tonydnguyen7 · 2024-10-30T13:40:45Z

Would love for this to be added in! Hopefully this isn't stale. @JoelSpeed

dextercai · 2024-12-30T03:08:45Z

hope to have this feature.

Add support for per route authorization

f931662

ibizaman requested a review from a team as a code owner February 11, 2023 10:03

ibizaman mentioned this pull request Apr 12, 2023

Option for per-route group authorization. #1966

Closed

github-actions bot added the Stale label Apr 18, 2023

Merge branch 'master' into route_auth

113f95e

JoelSpeed removed the Stale label Apr 19, 2023

tolleiv mentioned this pull request May 9, 2023

allowed_group for url paths #2073

Closed

sebwills reviewed May 10, 2023

View reviewed changes

github-actions bot added the Stale label Jul 12, 2023

github-actions bot removed the Stale label Jul 16, 2023

tuunit added this to the v8.0.0 milestone Sep 13, 2023

tuunit mentioned this pull request Oct 30, 2023

Restrict access to subpages with Azure AD group objectId with "allowed-group" and Nginx #2249

Closed

github-actions bot added the Stale label Nov 13, 2023

github-actions bot removed the Stale label Nov 15, 2023

github-actions bot added the Stale label Jan 31, 2024

github-actions bot removed the Stale label Feb 2, 2024

github-actions bot added the Stale label Apr 3, 2024

github-actions bot removed the Stale label Apr 6, 2024

github-actions bot added the Stale label Jun 5, 2024

github-actions bot closed this Jun 15, 2024

tuunit reopened this Jun 23, 2024

github-actions bot added go tests provider and removed Stale labels Jun 23, 2024

github-actions bot added the Stale label Aug 28, 2024

github-actions bot closed this Sep 4, 2024

tuunit reopened this Sep 4, 2024

tuunit removed the Stale label Sep 4, 2024

tuunit removed this from the v8.0.0 milestone Jan 4, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for per route authorization #2011

Add support for per route authorization #2011

ibizaman commented Feb 11, 2023

github-actions bot commented Apr 18, 2023

ibizaman commented Apr 19, 2023

sebwills May 10, 2023

ibizaman May 11, 2023

sebwills May 11, 2023

ibizaman May 12, 2023

sebwills commented May 10, 2023

github-actions bot commented Jul 12, 2023

ibizaman commented Jul 15, 2023

ghmer commented Jul 15, 2023

henriduflot commented Sep 4, 2023

github-actions bot commented Nov 13, 2023

ibizaman commented Nov 13, 2023 •

edited

Loading

igops commented Dec 1, 2023

github-actions bot commented Jan 31, 2024

haeraeus commented Jan 31, 2024

github-actions bot commented Apr 3, 2024

sebwills commented Apr 5, 2024

ibizaman commented Apr 5, 2024

github-actions bot commented Jun 5, 2024

pushc6 commented Jun 25, 2024

github-actions bot commented Aug 28, 2024

faresismail96 commented Sep 8, 2024

ctschubel commented Sep 25, 2024

tonydnguyen7 commented Oct 30, 2024

dextercai commented Dec 30, 2024

Add support for per route authorization #2011

Are you sure you want to change the base?

Add support for per route authorization #2011

Conversation

ibizaman commented Feb 11, 2023

Description

Motivation and Context

How Has This Been Tested?

Checklist:

github-actions bot commented Apr 18, 2023

ibizaman commented Apr 19, 2023

sebwills May 10, 2023

Choose a reason for hiding this comment

ibizaman May 11, 2023

Choose a reason for hiding this comment

sebwills May 11, 2023

Choose a reason for hiding this comment

ibizaman May 12, 2023

Choose a reason for hiding this comment

sebwills commented May 10, 2023

github-actions bot commented Jul 12, 2023

ibizaman commented Jul 15, 2023

ghmer commented Jul 15, 2023

henriduflot commented Sep 4, 2023

github-actions bot commented Nov 13, 2023

ibizaman commented Nov 13, 2023 • edited Loading

igops commented Dec 1, 2023

github-actions bot commented Jan 31, 2024

haeraeus commented Jan 31, 2024

github-actions bot commented Apr 3, 2024

sebwills commented Apr 5, 2024

ibizaman commented Apr 5, 2024

github-actions bot commented Jun 5, 2024

pushc6 commented Jun 25, 2024

github-actions bot commented Aug 28, 2024

faresismail96 commented Sep 8, 2024

ctschubel commented Sep 25, 2024

tonydnguyen7 commented Oct 30, 2024

dextercai commented Dec 30, 2024

ibizaman commented Nov 13, 2023 •

edited

Loading