From 4129ae3b5727b7f3c0737e4af6f32871d6be03b0 Mon Sep 17 00:00:00 2001 From: Igor Beliakov Date: Thu, 15 Jul 2021 21:12:56 +0300 Subject: [PATCH 1/7] Draft implementation of app roles for AzureAD Signed-off-by: Igor Beliakov --- oauthenticator/azuread.py | 31 ++++++++++++++++++++++++++++--- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/oauthenticator/azuread.py b/oauthenticator/azuread.py index bd7bb9ad..23fedfae 100644 --- a/oauthenticator/azuread.py +++ b/oauthenticator/azuread.py @@ -9,6 +9,7 @@ from jupyterhub.auth import LocalAuthenticator from tornado.httpclient import HTTPRequest from traitlets import default +from traitlets import List from traitlets import Unicode from .oauth2 import OAuthenticator @@ -19,6 +20,10 @@ PYJWT_2 = V(jwt.__version__) >= V("2.0") +def check_user_has_role(member_roles, allowed_roles): + return any(role in member_roles for role in allowed_roles) + + class AzureAdOAuthenticator(OAuthenticator): login_service = Unicode( os.environ.get('LOGIN_SERVICE', 'Azure AD'), @@ -38,6 +43,18 @@ def _tenant_id_default(self): def _username_claim_default(self): return 'name' + admin_azure_app_roles = List( + Unicode(), + config=True, + help="App roles that should give Jupyterhub admin privileges to a user", + ) + + allowed_azure_app_roles = List( + Unicode(), + config=True, + help="Automatically allow users with selected app roles", + ) + @default("authorize_url") def _authorize_url_default(self): return 'https://login.microsoftonline.com/{0}/oauth2/authorize'.format( @@ -80,9 +97,7 @@ async def authenticate(self, handler, data=None): if PYJWT_2: decoded = jwt.decode( - id_token, - options={"verify_signature": False}, - audience=self.client_id, + id_token, options={"verify_signature": False}, audience=self.client_id, ) else: # pyjwt 1.x @@ -94,6 +109,16 @@ async def authenticate(self, handler, data=None): # results in a decoded JWT for the user data auth_state['user'] = decoded + roles = decoded.get("roles", []) + + if self.admin_azure_app_roles: + if check_user_has_role(roles, self.admin_azure_app_roles): + userdict["admin"] = True + + if self.allowed_azure_app_roles: + if not check_user_has_role(roles, self.allowed_azure_app_roles): + return None + return userdict From 4fd4fb990a8cfae93866d5bfef0cbf7e2d19dfaf Mon Sep 17 00:00:00 2001 From: Igor Beliakov Date: Wed, 18 Aug 2021 23:52:35 +0200 Subject: [PATCH 2/7] Reformat azuread.py Signed-off-by: Igor Beliakov --- oauthenticator/azuread.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/oauthenticator/azuread.py b/oauthenticator/azuread.py index 23fedfae..5e46d5d7 100644 --- a/oauthenticator/azuread.py +++ b/oauthenticator/azuread.py @@ -97,7 +97,9 @@ async def authenticate(self, handler, data=None): if PYJWT_2: decoded = jwt.decode( - id_token, options={"verify_signature": False}, audience=self.client_id, + id_token, + options={"verify_signature": False}, + audience=self.client_id, ) else: # pyjwt 1.x From 6dc41a6df1e479a7a7048b7ddc49630319da40ea Mon Sep 17 00:00:00 2001 From: Igor Beliakov Date: Thu, 19 Aug 2021 14:50:25 +0200 Subject: [PATCH 3/7] Multiple changes: 1. Let admins login without explicitly having an allowed app role; 2. Style change for getting a list of roles; 3. Moved to Set traitlets. Signed-off-by: Igor Beliakov --- oauthenticator/azuread.py | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/oauthenticator/azuread.py b/oauthenticator/azuread.py index 5e46d5d7..6a644e72 100644 --- a/oauthenticator/azuread.py +++ b/oauthenticator/azuread.py @@ -9,7 +9,7 @@ from jupyterhub.auth import LocalAuthenticator from tornado.httpclient import HTTPRequest from traitlets import default -from traitlets import List +from traitlets import Set from traitlets import Unicode from .oauth2 import OAuthenticator @@ -43,16 +43,13 @@ def _tenant_id_default(self): def _username_claim_default(self): return 'name' - admin_azure_app_roles = List( - Unicode(), + admin_azure_app_roles = Set( config=True, help="App roles that should give Jupyterhub admin privileges to a user", ) - allowed_azure_app_roles = List( - Unicode(), - config=True, - help="Automatically allow users with selected app roles", + allowed_azure_app_roles = Set( + config=True, help="Automatically allow users with selected app roles", ) @default("authorize_url") @@ -111,11 +108,12 @@ async def authenticate(self, handler, data=None): # results in a decoded JWT for the user data auth_state['user'] = decoded - roles = decoded.get("roles", []) + roles = auth_state['user'].get("roles", []) if self.admin_azure_app_roles: if check_user_has_role(roles, self.admin_azure_app_roles): userdict["admin"] = True + return userdict if self.allowed_azure_app_roles: if not check_user_has_role(roles, self.allowed_azure_app_roles): From 4d535d6d04341d3c5fbcd9c467d2169593345d53 Mon Sep 17 00:00:00 2001 From: Igor Beliakov Date: Thu, 19 Aug 2021 17:58:50 +0200 Subject: [PATCH 4/7] Multiple changes: * (Optionally) Revoke stale admin privileges; * Improve help messages. Signed-off-by: Igor Beliakov --- oauthenticator/azuread.py | 32 ++++++++++++++++++++++++-------- 1 file changed, 24 insertions(+), 8 deletions(-) diff --git a/oauthenticator/azuread.py b/oauthenticator/azuread.py index 6a644e72..a6a4a869 100644 --- a/oauthenticator/azuread.py +++ b/oauthenticator/azuread.py @@ -9,6 +9,7 @@ from jupyterhub.auth import LocalAuthenticator from tornado.httpclient import HTTPRequest from traitlets import default +from traitlets import Bool from traitlets import Set from traitlets import Unicode @@ -43,13 +44,24 @@ def _tenant_id_default(self): def _username_claim_default(self): return 'name' - admin_azure_app_roles = Set( + app_roles_help_message = "More details on app roles: https://docs.microsoft.com/en-us/azure/active-directory/develop/howto-add-app-roles-in-azure-ad-apps" + + admin_users_app_roles = Set( + config=True, + help="Users with one of these app roles in their identity token claim 'roles' are treated as admins. " + + app_roles_help_message, + ) + + allowed_users_app_roles = Set( config=True, - help="App roles that should give Jupyterhub admin privileges to a user", + help="Only users with one of these app roles in their identity token claim 'roles' will be allowed to login into JupyterHub. Admins do not fall under this restriction. " + + app_roles_help_message, ) - allowed_azure_app_roles = Set( - config=True, help="Automatically allow users with selected app roles", + revoke_stale_admin_privileges = Bool( + False, + config=True, + help="If a user is not listed in admin_users or doesn't have an admin app role (admin_users_app_roles) in a token, user's admin privileges will be revoked upon login.", ) @default("authorize_url") @@ -110,13 +122,17 @@ async def authenticate(self, handler, data=None): roles = auth_state['user'].get("roles", []) - if self.admin_azure_app_roles: - if check_user_has_role(roles, self.admin_azure_app_roles): + # Strip stale admin privileges + if userdict["name"].lower() not in self.admin_users: + userdict["admin"] = False + + if self.admin_users_app_roles: + if check_user_has_role(roles, self.admin_users_app_roles): userdict["admin"] = True return userdict - if self.allowed_azure_app_roles: - if not check_user_has_role(roles, self.allowed_azure_app_roles): + if self.allowed_users_app_roles: + if not check_user_has_role(roles, self.allowed_users_app_roles): return None return userdict From 22626f542bdfce159df19c3075270241b5551d57 Mon Sep 17 00:00:00 2001 From: Igor Beliakov Date: Thu, 19 Aug 2021 18:01:05 +0200 Subject: [PATCH 5/7] Reorder imports Signed-off-by: Igor Beliakov --- oauthenticator/azuread.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/oauthenticator/azuread.py b/oauthenticator/azuread.py index a6a4a869..b3bc7c69 100644 --- a/oauthenticator/azuread.py +++ b/oauthenticator/azuread.py @@ -8,8 +8,8 @@ import jwt from jupyterhub.auth import LocalAuthenticator from tornado.httpclient import HTTPRequest -from traitlets import default from traitlets import Bool +from traitlets import default from traitlets import Set from traitlets import Unicode @@ -61,7 +61,7 @@ def _username_claim_default(self): revoke_stale_admin_privileges = Bool( False, config=True, - help="If a user is not listed in admin_users or doesn't have an admin app role (admin_users_app_roles) in a token, user's admin privileges will be revoked upon login.", + help="If a user is not listed in admin_users or doesn't have an admin app role (admin_users_app_roles) in a token, user's admin privileges will be revoked upon login", ) @default("authorize_url") @@ -122,9 +122,11 @@ async def authenticate(self, handler, data=None): roles = auth_state['user'].get("roles", []) - # Strip stale admin privileges - if userdict["name"].lower() not in self.admin_users: - userdict["admin"] = False + # Admin privileges will be revoked if a user is not listed in admin_users + # and then added back if the user has one of admin_users_app_roles. + if self.revoke_stale_admin_privileges: + if userdict["name"].lower() not in self.admin_users: + userdict["admin"] = False if self.admin_users_app_roles: if check_user_has_role(roles, self.admin_users_app_roles): From eab9ac870ba2385bdf74cdda16e1d47c15601114 Mon Sep 17 00:00:00 2001 From: Igor Beliakov Date: Fri, 20 Aug 2021 15:39:13 +0200 Subject: [PATCH 6/7] Removed the "revoke_stale_admin_privileges" feature Signed-off-by: Igor Beliakov --- oauthenticator/azuread.py | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/oauthenticator/azuread.py b/oauthenticator/azuread.py index b3bc7c69..36511f3e 100644 --- a/oauthenticator/azuread.py +++ b/oauthenticator/azuread.py @@ -8,7 +8,6 @@ import jwt from jupyterhub.auth import LocalAuthenticator from tornado.httpclient import HTTPRequest -from traitlets import Bool from traitlets import default from traitlets import Set from traitlets import Unicode @@ -58,12 +57,6 @@ def _username_claim_default(self): + app_roles_help_message, ) - revoke_stale_admin_privileges = Bool( - False, - config=True, - help="If a user is not listed in admin_users or doesn't have an admin app role (admin_users_app_roles) in a token, user's admin privileges will be revoked upon login", - ) - @default("authorize_url") def _authorize_url_default(self): return 'https://login.microsoftonline.com/{0}/oauth2/authorize'.format( @@ -122,12 +115,6 @@ async def authenticate(self, handler, data=None): roles = auth_state['user'].get("roles", []) - # Admin privileges will be revoked if a user is not listed in admin_users - # and then added back if the user has one of admin_users_app_roles. - if self.revoke_stale_admin_privileges: - if userdict["name"].lower() not in self.admin_users: - userdict["admin"] = False - if self.admin_users_app_roles: if check_user_has_role(roles, self.admin_users_app_roles): userdict["admin"] = True From cd98b927aaceefbb43c8c54d8d4d22c458ce7dc3 Mon Sep 17 00:00:00 2001 From: Igor Beliakov Date: Fri, 20 Aug 2021 18:27:20 +0200 Subject: [PATCH 7/7] Removed check_user_has_role() Signed-off-by: Igor Beliakov --- oauthenticator/azuread.py | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/oauthenticator/azuread.py b/oauthenticator/azuread.py index 36511f3e..f3336aed 100644 --- a/oauthenticator/azuread.py +++ b/oauthenticator/azuread.py @@ -20,10 +20,6 @@ PYJWT_2 = V(jwt.__version__) >= V("2.0") -def check_user_has_role(member_roles, allowed_roles): - return any(role in member_roles for role in allowed_roles) - - class AzureAdOAuthenticator(OAuthenticator): login_service = Unicode( os.environ.get('LOGIN_SERVICE', 'Azure AD'), @@ -113,15 +109,15 @@ async def authenticate(self, handler, data=None): # results in a decoded JWT for the user data auth_state['user'] = decoded - roles = auth_state['user'].get("roles", []) + roles = set(auth_state['user'].get("roles", [])) if self.admin_users_app_roles: - if check_user_has_role(roles, self.admin_users_app_roles): + if roles.intersection(self.admin_users_app_roles): userdict["admin"] = True return userdict if self.allowed_users_app_roles: - if not check_user_has_role(roles, self.allowed_users_app_roles): + if not roles.intersection(self.allowed_users_app_roles): return None return userdict