8000 Cookie-size overflow when using external (REST) role mapping · Issue #2111 · tchiotludo/akhq · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Cookie-size overflow when using external (REST) role mapping #2111

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers 8000 and the community.

Sign up for GitHub

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

Jump to bottom
Open
sverrehu opened this issue Mar 11, 2025 · 20 comments · May be fixed by #2125
Open

Cookie-size overflow when using external (REST) role mapping #2111

sverrehu opened this issue Mar 11, 2025 · 20 comments · May be fixed by #2125
Assignees
@AlexisSouquiere

Comments

@sverrehu
Copy link
Contributor

We have implemented a REST-based role-mapper.

It fails for users who have access to a large number of topics, because the JWT is rewritten to contain all the mapped groups, which in turn contain a large number of patterns for our topics.

The JWT is further set in a cookie, and cookies are typically limited to about 4 KiB. Even though the content is gzipped and BASE64-encoded, the size by far exceeds 4 KiB in our clusters.

We think it would be possible to keep the mapping server-side, and look it up based on the original JWT.
@github-project-automation github-project-automation bot moved this to Backlog in Backlog Mar 11, 2025
@sverrehu sverrehu mentioned this issue Mar 11, 2025
Merged
@AlexisSouquiere
Copy link
Collaborator

@sverrehu I agree that the 4KB cookie limit is a constraint with the fine-grained permissions that we have now.
Implementing an in-memory caching, is it what you are thinking about ? I'm not talking about distributed caching or anything else to keep AKHQ autonomous.

@sverrehu
Copy link
Contributor Author

Yes, in-memory cache is what I was thinking about.

@AlexisSouquiere AlexisSouquiere self-assigned this Mar 14, 2025
@erikgb
Copy link
Contributor
erikgb commented Mar 20, 2025

@sverrehu, are you preparing a PR to fix this? AFAIK your proposed solution works from a fork in our environment.

@AlexisSouquiere
Copy link
Collaborator

I started to work on it. The main concern that I have right now is about multiple AKHQ replicas. Suppose you have 2 AKHQ replicas in K8S or 2 AKHQ behind a LB, a request can be handled by one or the other replica. So this local cache needs to be set on the replica that doesn't handle the login.
The SecurityRule needs to be able to retrieve the user's group from the selected provider (basic, ldap, oidc, ...), which is not supported currently

@sverrehu
Copy link
Contributor Author
sverrehu commented Mar 20, 2025 •
edited
Loading

I haven't gotten around to making it a proper PR yet, but this is what I have, that solves the problem at our site:
https://github.com/sverrehu/akhq/pull/1/files
It should work across replicas, as it uses the original JWT to look up the groups (via the ClaimProvider) when the cookie comes in from the user's browser.
The basic idea is not to map the groups during (OIDC) login, but rather delay the lookup until later, without passing the mapped groups to the user.

@sverrehu sverrehu linked a pull request Mar 20, 2025 that will close this issue
Open
@sverrehu
Copy link
Contributor Author

Update: I made the above mentioned, local PR into a draft PR: #2125
I guess there will be discussions.

@AlexisSouquiere
Copy link
Collaborator

I understand what you did to make OIDC works in your case. The thing is that storing the groups in the JWT token to process them after won't work for:

as these 2 give directly the groups mapping (role/pattern/cluster) and not just the AKHQ group to map with the configuration.

@erikgb
Copy link
Contributor
erikgb commented Mar 21, 2025

@AlexisSouquiere, I think you should look closer at the PR. 👓

akhq/src/main/java/org/akhq/security/mapper/OidcUserDetailsMapper.java

Lines 60 to 62 in f3dea5a

if(provider.isUseOidcClaim()){
return (Flowable.just(createDirectClaimAuthenticationResponse(oidcUsername, openIdClaims)));
}

is still present. And if the provider_name claim is NOT present in the token, the groups claim will be used as-is.

@AlexisSouquiere
Copy link
Collaborator

I saw this 😄 I was more thinking of a way to avoid completely the groups mapping from the JWT token, even for the direct OIDC mapping and the external mapping

@erikgb
Copy link
Contributor
erikgb commented Mar 21, 2025 •
edited
Loading

Yes, this should probably be reworked, but I am thinking short-term and long-term fixes. We cannot use AKHQ without an immediate fix for this.

@sverrehu
Copy link
Contributor Author

I fixed the tests now. PTAL, @AlexisSouquiere

@AlexisSouquiere
Copy link
Collaborator

I will check that before Thursday EOD. At first glance the PR looks good.
Just want to do some tests including with the other authentication methods

@sverrehu
Copy link
Contributor Author

Thanks!

@AlexisSouquiere
Copy link
Collaborator
AlexisSouquiere commented Mar 27, 2025 •
edited
Loading

I did the review and with small changes, your proposal works.

After testing, the only concern that I have is that now using OIDC + the External claim provider, on each and every request, the external claim provider will be called. It's an impacting change as we were only doing a single call preivously. I think that setting up a local cache with caffeine is kind of mandatory to avoid this behaviour. WDYT ?

With OIDC + groups mapping from the config there is no issues as we are only using local data, but with the external claim provider, the situation has changed

@sverrehu
Copy link
Contributor Author

Thanks for your review and your comments! I have a branch in which I try to implement caching, but I have not tested it yet:
70107b6

@doxsch
Copy link
Contributor
doxsch commented Apr 15, 2025

Hello everyone,

I just wanted to quickly ask how things will proceed here. We would be happy if we could use this feature. Is there a timeline for when we can expect it?

Thank you very much for your efforts.

@sverrehu
Copy link
Contributor Author

@AlexisSouquiere we tested the Caffeine-based caching locally now, and it makes everything a lot faster. Maybe I should merge it into the main PR?

@AlexisSouquiere
Copy link
Collaborator

@sverrehu I think I missed this comment 😢 Yes I think it would be better to have it in the same PR

tchiotludo pushed a commit that referenced this issue May 12, 2025
@sverrehu
chore(auth): warn when encoded groups may not fit inside cookie (#2112)
7f197f0 
related to #2111
@sverrehu
Copy link
Contributor Author

@sverrehu I think I missed this comment 😢 Yes I think it would be better to have it in the same PR

I merged the caching into the main PR no. PTAL, @AlexisSouquiere !

@AlexisSouquiere
Copy link
Collaborator

Thanks ! I'm working on it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@AlexisSouquiere AlexisSouquiere

Labels
None yet
Projects
Backlog
Status: Backlog
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

chore: do not pass externally mapped groups in token sverrehu/akhq
4 participants
@erikgb @AlexisSouquiere @sverrehu @doxsch
0