Issue #1102 - Add config to explicitly enable loading all github groups#1349
Conversation
|
My github has a lot of private organizations that aren't relevant to my employer. What's the use case for needing orgs that you can't enumerate beforehand? |
|
@ericchiang I assume your question related to #1340 , right? Use-case is the following: let any github user access the application; give read-only to everyone and give admin permissions if the user belongs to some team/org. Before #1340 I would have to list all github groups which is not possible. I hope I understood your question correctly. |
srenatus
left a comment
There was a problem hiding this comment.
Looks good to me.
I'm curious, did you employ some sort of test suite that made this apparent in your system (against master), or did you just examine changes and notice this potential issue? 😃
I work with @alexmt. What happened was, while attempting to roll this version of dex out to production, we encountered one user who belonged to so many github orgs/teams, that the resulting JWT token exceeded the max HTTP cookie size of 4093 bytes, and so the user couldn't log in (despite observing proper OAuth2 login flow). That is when we realized that this change could break existing dex users. That was a fun one to debug. |
|
Eek -- I had hinted at that risk here, #1345 (review), but probably neither prominent nor early enough for y'alls. |
this was made optional via dexidp/dex#1349
…oups Add config to explicitly enable loading all github groups Follow-up for dexidp#1102.
I've discovered that new behavior implemented in #1340 might be undesired for some users. The user might be part of multiple orgs and teams, so if all groups are loaded then JWT token does not fit into cookie.
With changes from #1340 applications which uses Dex + github without configuring orgs now might get a token which does not fit into cookie. Although it is a rare case I think it worth adding new setting which explicitly enable github connector to load all user groups.