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

Add mechanism to easily restrict access to users that meet given conditions #26

Open
holyjak opened this issue Dec 26, 2019 · 8 comments
Labels
enhancement New feature or request

Comments

@holyjak
Copy link

holyjak commented Dec 26, 2019

Hello, thank you very much for this great piece of work!

I have a question: I want to allow access only for members of a particular GH Team. Is it possible? How? Thank you!!!

@TimothyJones
Copy link
Owner

Yes, this is possible with some extra development effort. It depends a bit on your use case.

In another project, I added the github organisation / team metadata to the userinfo response, which the app then read to determine whether access was appropriate.

Another approach could be to add a callback to the wrapper, which would be called before the cognito callback. This callback would check the teams, and either redirect to a "login failed" page on failure, or redirect to the cognito callback on success. If you go this route, a PR would be welcome (the functionality could be generally useful).

@TimothyJones TimothyJones added the enhancement New feature or request label Jan 3, 2020
@holyjak
Copy link
Author

holyjak commented Jan 11, 2020 via email

@TimothyJones TimothyJones changed the title Q: How to lock access down only to a particular GH Team? Add mechanism to easily restrict access to users that meet given conditions Jan 16, 2020
@TimothyJones
Copy link
Owner

No problem. I'll leave the ticket open, because adding a way to easily add this functionality would be useful.

@lolejar
Copy link

lolejar commented Feb 6, 2020

just for info Timothy could you possibly merge that functionality with team/organization into this project ? i am looking into this for some time and since you already did it i would like not to reinvent a wheel. or just point me to some code where this is already done (if it is public). thank you so much.

@lolejar
Copy link

lolejar commented Feb 17, 2020

sorry for bumping up but can someone help me / guide me on request above ?

@TimothyJones
Copy link
Owner

So, it depends where you do the restriction.

If you control the server(s) where you want to use your user pool tokens, then extending the userinfo to include custom claims (such as organisation) is fairly straightforward - you would do so by adding another call to the promise array here. Then, you could map the custom claim to an attribute in your user pool, and read that claim on your target server to determine whether or not to allow access. With this pattern, github users outside the organisation(s) can successfully use github to authenticate, but are unable to use their tokens to do anything. This is what I did when I implemented this feature, but because most of the code is on the server side (and out of the scope of this repository), it doesn't make sense to include it here.

If you want to prevent users outside the organisation from authenticating to the userpool at all, then you would need to add a callback URL to intercept the callback from GitHub (the wrapper currently doesn't have a callback URL, it passes the callback straight to Cognito). This callback URL would need to exchange tokens with github, call the organisations endpoint in github's API, and then determine whether to redirect back to Cognito's callback, or to a page that communicates the failure. If you go this route, I think you would also have to implement your own tokens, since cognito will want to complete the code flow. You'd also need to do something like store the github token on the token you generate, so that you can unpack it and use it when Cognito calls the userinfo endpoint.

The second option is considerably more fiddly, which is why I went with the first option when I was faced with this problem.

And of course, if you (or anyone else) sees an easier way of implementing this within this wrapper, I'd welcome suggestions and/or pull requests.

@ibtehajn
Copy link

Thanks for the explanation and this super useful project. I am facing the same problem and was going to implement the first option above. Your comment gives me confidence that this approach can work.

@TimothyJones
Copy link
Owner

You’re very welcome- I’m glad it helps!

If there’s anything I can do in the main repo to make your life easier, do let me know (and of course PRs are welcome if you end up making improvements that might be generally useful)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants