search

heliumdatacommons / auth_microservice

Microservice which abstracts out OAuth2/OpenID exchanges and token management from applications
4 stars 5 forks source link

More changes for py27 and el7 rpms #18

Closed stdweird closed 6 years ago

stdweird commented 6 years ago

@theferrit32 i've started testing, but i'm not finished. i apologise for once again having to mess around with the structure, but what was under example/microservice is the actual app, not just some example.

the readme has not been adapted yet to the new structure.

stdweird commented 6 years ago

@theferrit32 this PR has a set of print replaced by logging, which leads to easy conflicts with some of the cleanup work you are doing.

stdweird commented 6 years ago

@theferrit32 wrt https://github.com/heliumdatacommons/auth_microservice/pull/18/commits/cf55b08a5daacf7b01c6c89ee349557738b676a0 i ran into a introspection configuration issue, which is provided as part of the metadata for our openid endpoint. so refactored the way certain data is read from the configuration in redirect_handler.

i still have one question: it looks like in part of the code you expect the intropsection_endpoint to be templatable (https://github.com/heliumdatacommons/auth_microservice/pull/18/files#diff-d73b6480b289bb64e56658e5f50591d2R315; but same code exists in master); however, for the google validator you do string concatenation (see https://github.com/heliumdatacommons/auth_microservice/pull/18/files#diff-d73b6480b289bb64e56658e5f50591d2L589)

stdweird commented 6 years ago

@theferrit32 i'm able to use SSO with irods now. i need some more changes, but they are more related to our openid setup than el7 i will make separate PRs for that

stdweird commented 6 years ago

@theferrit32 rebased on master (some of the code was refactored, so most changes you make will have conflict with this PR)

theferrit32 commented 6 years ago

Hi @stdweird I have been in parallel working on integrating some services including this with the Auth0 identity provider in the last few days and have gotten it working today. I will look and see how serious the conflicts in this PR are tomorrow. Most of the changes I made were in redirect_handler.py (which really needs to be renamed at this point) and views.py. There was also a schema change in models.py.

theferrit32 commented 6 years ago

It looks like the conflicts are not that substantial, a few functions moved, you refactored some code duplication, and changed some logging. I also changed the User.id field to User.sub, so when resolving conflicts on those lines it is important to merge both changes to the same line. If you are able to resolve these before I get a chance to look at it I will deploy on my dev server to test.

stdweird commented 6 years ago

@theferrit32 i'll fix the conflicts, probably adding some more cleanup/refactoring too.

stdweird commented 6 years ago

@theferrit32 rebased, but not retested if you can enable the travis unittests, you'll see that they will fail due to unknown variable token_endpoint at https://github.com/heliumdatacommons/auth_microservice/blob/a017b2d4af3c5c0b5350a835aaf105e4a0c00c88/token_service/redirect_handler.py#L670 should be easy to fix with token_endpoint = get_provider_config(provider, 'token_endpoint')

once this is merged, i'll make a new PR with further cleanup and some new configuration features we need

stdweird commented 6 years ago

Replaced by #21