Skip to content

Commit

Permalink
Verify client scopes before accepting token
Browse files Browse the repository at this point in the history
  • Loading branch information
nuwang committed Nov 6, 2023
1 parent 3d1244d commit 517f0e7
Show file tree
Hide file tree
Showing 7 changed files with 69 additions and 16 deletions.
9 changes: 6 additions & 3 deletions lib/galaxy/authnz/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,15 +88,18 @@ def logout(self, trans, post_user_logout_href=None):
"""
raise NotImplementedError()

def find_user_by_access_token(self, sa_session, access_token):
def decode_user_access_token(self, sa_session, access_token):
"""
Locates a user by access_token. The access token must be verified prior
to returning the relevant user.
Verifies and decodes an access token against this provider, returning the user and
a dict containing the decoded token data.
:type sa_session: sqlalchemy.orm.scoping.scoped_session
:param sa_session: SQLAlchemy database handle.
:type access_token: string
:param access_token: An OIDC access token
:return: A tuple containing the user and decoded jwt data
:rtype: Tuple[User, dict]
"""
raise NotImplementedError()
17 changes: 15 additions & 2 deletions lib/galaxy/authnz/custos_authnz.py
Original file line number Diff line number Diff line change
Expand Up @@ -502,7 +502,19 @@ def _username_from_userinfo(trans, userinfo):
else:
return username

def find_user_by_access_token(self, sa_session, access_token):
def decode_user_access_token(self, sa_session, access_token):
"""Verifies and decodes an access token against this provider, returning the user and
a dict containing the decoded token data.
:type sa_session: sqlalchemy.orm.scoping.scoped_session
:param sa_session: SQLAlchemy database handle.
:type access_token: string
:param access_token: An OIDC access token
:return: A tuple containing the user and decoded jwt data
:rtype: Tuple[User, dict]
"""
if not self.jwks_client:
return None
signing_key = self.jwks_client.get_signing_key_from_jwt(access_token)
Expand All @@ -524,7 +536,8 @@ def find_user_by_access_token(self, sa_session, access_token):
# jwt verified, we can now fetch the user
user_id = decoded_jwt["sub"]
custos_authnz_token = self._get_custos_authnz_token(sa_session, user_id, self.config.provider)
return custos_authnz_token.user if custos_authnz_token else None
user = custos_authnz_token.user if custos_authnz_token else None
return user, decoded_jwt


class OIDCAuthnzBaseKeycloak(OIDCAuthnzBase):
Expand Down
22 changes: 19 additions & 3 deletions lib/galaxy/authnz/managers.py
Original file line number Diff line number Diff line change
Expand Up @@ -408,16 +408,32 @@ def create_user(self, provider, token, trans, login_redirect_url):
log.exception(msg)
return False, msg, (None, None)

def _find_user_by_access_token_in_provider(self, sa_session, provider, access_token):
def _assert_jwt_contains_scopes(self, user, jwt, required_scopes):
if not jwt:
raise exceptions.AuthenticationFailed(
err_msg=f"User: {user.username} does not have the required scopes: [{required_scopes}]"
)
scopes = jwt.get("scope") or ""
if not set(required_scopes).issubset(scopes.split(" ")):
raise exceptions.AuthenticationFailed(
err_msg=f"User: {user.username} has JWT with scopes: [{scopes}] but not required scopes: [{required_scopes}]"
)

def _validate_permissions(self, user, jwt):
required_scopes = [f"{self.app.config.oidc_scope_prefix}:*"]
self._assert_jwt_contains_scopes(user, jwt, required_scopes)

def _match_access_token_to_user_in_provider(self, sa_session, provider, access_token):
try:
success, message, backend = self._get_authnz_backend(provider)
if success is False:
msg = f"An error occurred when obtaining user by token with provider `{provider}`: {message}"
log.error(msg)
return None
user = backend.find_user_by_access_token(sa_session, access_token)
user, jwt = backend.decode_user_access_token(sa_session, access_token)
if user:
log.debug(f"Found user: {user} via `{provider}` identity provider")
self._validate_permissions(user, jwt)
return user
return None
except NotImplementedError:
Expand All @@ -429,7 +445,7 @@ def _find_user_by_access_token_in_provider(self, sa_session, provider, access_to

def match_access_token_to_user(self, sa_session, access_token):
for provider in self.oidc_backends_config:
user = self._find_user_by_access_token_in_provider(sa_session, provider, access_token)
user = self._match_access_token_to_user_in_provider(sa_session, provider, access_token)
if user:
return user
return None
Expand Down
5 changes: 5 additions & 0 deletions lib/galaxy/config/sample/galaxy.yml.sample
Original file line number Diff line number Diff line change
Expand Up @@ -2169,6 +2169,11 @@ galaxy:
# <config_dir>.
#oidc_backends_config_file: oidc_backends_config.xml

# Sets the prefix for OIDC scopes specific to this Galaxy instance.
# If an API call is made against this Galaxy instance using an OIDC bearer token,
# it must include a scope with "<prefix>:*". e.g "https://galaxyproject.org/api:*"
#oidc_scope_prefix: https://galaxyproject.org/api

# XML config file that allows the use of different authentication
# providers (e.g. LDAP) instead or in addition to local authentication
# (.sample is used if default does not exist).
Expand Down
9 changes: 9 additions & 0 deletions lib/galaxy/config/schemas/config_schema.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2903,6 +2903,15 @@ mapping:
desc: |
Sets the path to OIDC backends configuration file.
oidc_scope_prefix:
type: str
default: https://galaxyproject.org/api
required: false
desc: |
Sets the prefix for OIDC scopes specific to this Galaxy instance.
If an API call is made against this Galaxy instance using an OIDC bearer token,
it must include a scope with "<prefix>:*". e.g "https://galaxyproject.org/api:*"
auth_config_file:
type: str
default: auth_conf.xml
Expand Down
10 changes: 5 additions & 5 deletions test/integration/oidc/galaxy-realm-export.json
Original file line number Diff line number Diff line change
Expand Up @@ -424,7 +424,7 @@
"clientScope" : "offline_access",
"roles" : [ "offline_access" ]
}, {
"clientScope" : "gx:*",
"clientScope" : "https://galaxyproject.org/api:*",
"roles" : [ "galaxy-access-role" ]
} ],
"clientScopeMappings" : {
Expand Down Expand Up @@ -567,7 +567,7 @@
"fullScopeAllowed" : true,
"nodeReRegistrationTimeout" : -1,
"defaultClientScopes" : [ "web-origins", "acr", "bpa.*", "profile", "roles", "email" ],
"optionalClientScopes" : [ "gx:*", "address", "phone", "offline_access", "microprofile-jwt" ]
"optionalClientScopes" : [ "https://galaxyproject.org/api:*", "address", "phone", "offline_access", "microprofile-jwt" ]
}, {
"id" : "4a0e0a29-e407-4154-94f3-a82d85ceff04",
"clientId" : "broker",
Expand Down Expand Up @@ -632,7 +632,7 @@
"authenticationFlowBindingOverrides" : { },
"fullScopeAllowed" : true,
"nodeReRegistrationTimeout" : -1,
"defaultClientScopes" : [ "web-origins", "acr", "profile", "roles", "email", "gx:*" ],
"defaultClientScopes" : [ "web-origins", "acr", "profile", "roles", "email", "https://galaxyproject.org/api:*" ],
"optionalClientScopes" : [ "address", "phone", "offline_access", "microprofile-jwt" ]
}, {
"id" : "d27406eb-c929-4658-904f-f42f8bd2812c",
Expand Down Expand Up @@ -1047,7 +1047,7 @@
} ]
}, {
"id" : "aabfb2e4-8718-4f21-a290-873729b9a64a",
"name" : "gx:*",
"name" : "https://galaxyproject.org/api:*",
"description" : "",
"protocol" : "openid-connect",
"attributes" : {
Expand Down Expand Up @@ -1259,7 +1259,7 @@
} ]
} ],
"defaultDefaultClientScopes" : [ "role_list", "profile", "email", "roles", "web-origins", "acr" ],
"defaultOptionalClientScopes" : [ "offline_access", "address", "phone", "microprofile-jwt", "gx:*" ],
"defaultOptionalClientScopes" : [ "offline_access", "address", "phone", "microprofile-jwt", "https://galaxyproject.org/api:*" ],
"browserSecurityHeaders" : {
"contentSecurityPolicyReportOnly" : "",
"xContentTypeOptions" : "nosniff",
Expand Down
13 changes: 10 additions & 3 deletions test/integration/oidc/test_auth_oidc.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
KEYCLOAK_ADMIN_PASSWORD = "admin"
KEYCLOAK_TEST_USERNAME = "gxyuser"
KEYCLOAK_TEST_PASSWORD = "gxypass"
KEYCLOAK_HOST_PORT = 9443
KEYCLOAK_HOST_PORT = 8443
KEYCLOAK_URL = f"https://localhost:{KEYCLOAK_HOST_PORT}/realms/gxyrealm"


Expand Down Expand Up @@ -128,7 +128,7 @@ def configure_oidc_and_restart(cls):

@classmethod
def tearDownClass(cls):
# stop_keycloak_docker(cls.container_name)
stop_keycloak_docker(cls.container_name)
cls.restoreOauthlibHttps()
os.remove(cls.backend_config_file)
super().tearDownClass()
Expand Down Expand Up @@ -250,7 +250,9 @@ def test_auth_with_expired_token(self):

def test_auth_with_another_authorized_client(self):
_, response = self._login_via_keycloak(KEYCLOAK_TEST_USERNAME, KEYCLOAK_TEST_PASSWORD)
access_token = self._get_keycloak_access_token(client_id="bpaclient", scopes=["gx:*"])
access_token = self._get_keycloak_access_token(
client_id="bpaclient", scopes=["https://galaxyproject.org/api:*"]
)
response = self._get("users/current", headers={"Authorization": f"Bearer {access_token}"})
self._assert_status_code_is(response, 200)
assert response.json()["email"] == "[email protected]"
Expand All @@ -266,3 +268,8 @@ def test_auth_with_unauthorized_client(self):
access_token = self._get_keycloak_access_token(client_id="unauthorizedclient")
response = self._get("users/current", headers={"Authorization": f"Bearer {access_token}"})
self._assert_status_code_is(response, 400)

def test_auth_without_required_scopes(self):
access_token = self._get_keycloak_access_token(client_id="bpaclient")
response = self._get("users/current", headers={"Authorization": f"Bearer {access_token}"})
self._assert_status_code_is(response, 400)

0 comments on commit 517f0e7

Please sign in to comment.