Authentication setup #316
Authentication setup #316
Conversation
src/cnaas_nms/api/auth.py
Outdated
| return oauth_client.connext.authorize_redirect(redirect_uri) | ||
| except Exception as e: | ||
| logger.error("Exception during authorized redirect: {}".format(str(e))) | ||
| return empty_result(status="error", data="Exception during authorized redirect."), 500 |
There was a problem hiding this comment.
Probably better to return a 401 and/or a 403 if you receive it from the auth service and otherwise a 503 temp unavailable as there is no real backend ISE that we can fix.
There was a problem hiding this comment.
I doubled checked and we can't get to this exception, unless our env variables are wrong so for now didn't catch the exception at all.
src/cnaas_nms/api/auth.py
Outdated
| token = oauth_client.connext.authorize_access_token() | ||
| except Exception as e: | ||
| logger.error("Exception during authorization of the access token: {}".format(str(e))) | ||
| return empty_result(status="error", data="Exception during authorization of the access token."), 500 |
There was a problem hiding this comment.
Same as above, probably best to handle the 401 and 403 case transparently and otherwise return a 503 as the auth service is probably down.
There was a problem hiding this comment.
We don't ever get a specific error code with this. The only error I could find is the MismatchingStateError which comes when the state is not matching (I can trigger this by calling this api call directly). I catched that specific error.
src/cnaas_nms/app_settings.py
Outdated
| OIDC_CONF_WELL_KNOWN_URL: str = "well-known-openid-configuration-endpoint" | ||
| OIDC_CLIENT_SECRET: str = "xxx" | ||
| OIDC_CLIENT_ID: str = "client-id" | ||
| FE_CALLBACK_URL: str = "http://localhost/callback" |
src/cnaas_nms/tools/security.py
Outdated
| except exceptions.JWKError: | ||
| try: | ||
| # with this exception, we first try to reload the keys | ||
| logger.info('JWT.decode didnt work. Get the keys and retry.') |
There was a problem hiding this comment.
Maybe move to debug level?
src/cnaas_nms/tools/security.py
Outdated
| try: | ||
| # with this exception, the call was too fast or the computer too slow | ||
| logger.info('Checked token too fast (or computer is too slow), retry') | ||
| sleep(1) |
There was a problem hiding this comment.
Probably your local machine ;) So I think we can remove this.
src/cnaas_nms/tools/security.py
Outdated
| if not api_settings.JWT_ENABLED: | ||
| return "admin" | ||
| # request the userinfo | ||
| url = auth_settings.OIDC_CONF_WELL_KNOWN_URL.split('.well-known')[0] + 'oidc/userinfo' |
There was a problem hiding this comment.
Might not be oidc/userinfo in all cases. This is retrievable by reading the .well-know configuration. so probably something likt this:
`
metadata = requests.get(settings.OIDC_WELL_KNOWN_ENDPOINT)
user_info_endpoint = metadata.json()["userinfo_endpoint"]
`
src/cnaas_nms/tools/security.py
Outdated
| try: | ||
| # with this exception, we first try to reload the keys | ||
| logger.info('JWT.decode didnt work. Get the keys and retry.') | ||
| response = requests.get(url=auth_settings.OIDC_CONF_WELL_KNOWN_URL.split('.well-known')[0] + 'oidc/certs') |
There was a problem hiding this comment.
Same as below, should be dynamic
There was a problem hiding this comment.
Thanks, changed it to follow your recommendation.
…for better naming
…Josephine-Rutten/cnaas-nms into feature.authenticationwithoauth
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## develop #316 +/- ##
===========================================
- Coverage 65.46% 64.61% -0.85%
===========================================
Files 66 67 +1
Lines 7325 7509 +184
===========================================
+ Hits 4795 4852 +57
- Misses 2530 2657 +127 ☔ View full report in Codecov by Sentry. |
|
SonarCloud Quality Gate failed.
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
I added two comments here, but I got this working with our Sunet SATOSA setup now :) |
src/cnaas_nms/api/app.py
Outdated
| server_metadata_url=auth_settings.OIDC_CONF_WELL_KNOWN_URL, | ||
| client_id=auth_settings.OIDC_CLIENT_ID, | ||
| client_secret=auth_settings.OIDC_CLIENT_SECRET, | ||
| client_kwargs={"scope": "openid"}, |
There was a problem hiding this comment.
For our satosa server to work with this we seem to need to modify this from just "openid" to "openid email" so we can get the email for logging etc later
There was a problem hiding this comment.
Great, I've added it to the app_setting, so it can be set as an env variable or in the auth_config.yaml
There was a problem hiding this comment.
Ah great! Can we add something about the new config options on this documentation page also? docs/configuration/index.rst
src/cnaas_nms/tools/security.py
Outdated
| if not auth_settings.OIDC_ENABLED: | ||
| return "Admin" | ||
| # Request the userinfo | ||
| metadata = requests.get(auth_settings.OIDC_CONF_WELL_KNOWN_URL) |
There was a problem hiding this comment.
Sorry, one more error that I think it would be good if we catch, if the metadata is unavailable when doing any API request the client will get back message: "Internal server error", if we catch requests.exceptions.ConnectionError here and return something like "OIDC metadata unavailable: ConnectionError" (or whatever error from str(e) )
There was a problem hiding this comment.
I had troubles reproducing the error, so I had troubles testing it. I catched the error you mentoined and another error I got instead. Let me know how you produced this error if the current changes are not catching the error you meant.
|
|
Tested with SATOSA builtin OIDC with opaque access_token and new https://github.com/UniversitaDellaCalabria/SATOSA-oidcop/tree/main using JWT access_token both working! |
Based on ticket: #310
We implemented the OAuth login in the backend.
We still need to make changes on the frontend. We also need to make a unit test for some functions.