Skip to content

Commit

Permalink
match new way for identifying iss in idpyoidc (merge commit)
Browse files Browse the repository at this point in the history
Merge branch 'bugfix/aai-conf' into 'main'
* match new way for identifying state & client type in idpyoidc

See merge request https://gitlab.ci.csc.fi/sds-dev/sd-submit/metadata-submitter/-/merge_requests/841

Approved-by: Joonatan Mäkinen <[email protected]>
Approved-by: Teemu Kataja <[email protected]>
Co-authored-by: Stefan Negru <[email protected]>
Merged by Teemu Kataja <[email protected]>
  • Loading branch information
teemukataja committed Feb 22, 2024
2 parents a8c3f1b + 971df8c commit b2c9e4e
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 11 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Updated BP image and dataset JSON schemas for better functionality #664
- Updated XML to JSON converter to parse ISO-8601 duration strings into numbers in BP sample attributes #696
- Updated middleware to accept user signed tokens
- make use of `idpyoidc` library instead of `oidc-rp`

### Removed

Expand Down
18 changes: 10 additions & 8 deletions metadata_backend/api/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ def __init__(self, aai: dict[str, Any]) -> None:
"issuer": self.iss,
"client_id": self.client_id,
"client_secret": self.client_secret,
"client_type": "oidc",
"redirect_uris": [self.callback_url],
"behaviour": {
"response_types": self.auth_method.split(" "),
Expand Down Expand Up @@ -85,16 +86,17 @@ async def login(self, _: Request) -> web.HTTPSeeOther:

# Generate authentication payload
try:
session = self.rph.begin("aai")
# this returns str even though begin mentions a dict
rph_session_url = self.rph.begin("aai")
except Exception as e:
# This can be caused if config is improperly configured, and
# idpyoidc is unable to fetch oidc configuration from the given URL
LOG.exception("OIDC authorization request failed with: %r", e)
raise web.HTTPInternalServerError(reason="OIDC authorization request failed.")

# Redirect user to AAI
response = web.HTTPSeeOther(session["url"])
response.headers["Location"] = session["url"]
response = web.HTTPSeeOther(rph_session_url)
response.headers["Location"] = rph_session_url
return response

async def callback(self, req: Request) -> Response:
Expand All @@ -120,20 +122,20 @@ async def callback(self, req: Request) -> Response:

# Verify oidc_state and retrieve auth session
try:
session = self.rph.get_session_information(params["state"])
session_info = self.rph.get_session_information(params["state"])
except KeyError as e:
# This exception is raised if the RPHandler doesn't have the supplied "state"
LOG.exception("Session not initialised, failed with: %r", e)
raise web.HTTPUnauthorized(reason="Bad user session.")

# Place authorization_code to session for finalize step
session["auth_request"]["code"] = params["code"]
session_info["code"] = params["code"]

# finalize requests id_token and access_token with code, validates them and requests userinfo data
try:
session = self.rph.finalize(session["iss"], session["auth_request"])
session = self.rph.finalize(self.iss, session_info)
except KeyError as e:
LOG.exception("Issuer: %s not found, failed with: %r.", session["iss"], e)
LOG.exception("Issuer: %s not found, failed with: %r.", session_info["iss"], e)
raise web.HTTPBadRequest(reason="Token issuer not found.")
except OidcMsgError as e:
# This exception is raised if RPHandler encounters an error due to:
Expand All @@ -153,7 +155,7 @@ async def callback(self, req: Request) -> Response:
# Create session
browser_session = await aiohttp_session.new_session(req)
browser_session["at"] = time.time()
browser_session["oidc_state"] = params["state"]
browser_session["oidc_state"] = session["state"]
browser_session["access_token"] = session["token"]
await self._set_user(req, browser_session, user_data)

Expand Down
8 changes: 5 additions & 3 deletions tests/unit/test_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ async def test_login_fail(self):

async def test_login_pass(self):
"""Test login redirects user."""
response = {"url": "some url"}
response = "url"
request = Mock_Request()
with patch("idpyoidc.client.rp_handler.RPHandler.begin", return_value=response):
response = await self.AccessHandler.login(request)
Expand All @@ -187,10 +187,11 @@ async def test_callback_pass_csc(self):
request.query["state"] = "state"
request.query["code"] = "code"

session = {"iss": "http://auth.domain.com:5430", "auth_request": {}}
session = {"iss": "http://auth.domain.com:5430", "code": "code"}
finalize = {
"token": "token",
"userinfo": {"sub": "user", "given_name": "name", "family_name": "name", "sdSubmitProjects": "1000 2000"},
"state": {},
}
db_client = MagicMock()
db_database = MagicMock()
Expand All @@ -212,7 +213,7 @@ async def test_callback_pass_ls(self):
request.query["state"] = "state"
request.query["code"] = "code"

session = {"iss": "http://auth.domain.com:5430", "auth_request": {}}
session = {"iss": "http://auth.domain.com:5430", "code": "code"}
finalize = {
"token": "token",
"userinfo": {
Expand All @@ -221,6 +222,7 @@ async def test_callback_pass_ls(self):
"family_name": "name",
"eduperson_entitlement": ["group1", "group2"],
},
"state": {},
}
db_client = MagicMock()
db_database = MagicMock()
Expand Down

0 comments on commit b2c9e4e

Please sign in to comment.