diff --git a/CHANGELOG.md b/CHANGELOG.md index 9df2fb214c117a471077517a176e2d51a16f2146..187d3dc59fe1bf12baae9fbd5d82dc59a1b91f3f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ - Clarify "set new password" button (#94) - Show error messages when login fails, for example when a wrong password was entered (#96) +- Fix access checking for monitoring (#105) ## [0.5.1] diff --git a/backend/Dockerfile b/backend/Dockerfile index ab8c019d9453fd29a900fcbde029425275ab1958..c62abf74718d46c6a1825032ce60e84b44f3b84f 100644 --- a/backend/Dockerfile +++ b/backend/Dockerfile @@ -11,7 +11,7 @@ COPY . . # hadolint ignore=DL3008 RUN apt-get update \ - && apt-get install --no-install-recommends -y gcc libffi-dev libc6-dev \ + && apt-get install --no-install-recommends -y gcc g++ libffi-dev libc6-dev \ && apt-get clean \ && rm -rf /var/lib/apt/lists/* \ && pip install --no-cache-dir -r requirements.txt diff --git a/backend/areas/apps/models.py b/backend/areas/apps/models.py index 083bcbd41f7d9b893837822f1712fedb945b94be..ef930501a9d8be66a5d27114531edd57477c2574 100644 --- a/backend/areas/apps/models.py +++ b/backend/areas/apps/models.py @@ -296,3 +296,21 @@ class AppStatus(): # pylint: disable=too-few-public-methods "ready": self.ready, "message": self.message, } + +class OAuthClientApp(db.Model): # pylint: disable=too-few-public-methods + """ + The OAuthClientApp object maps an OAuth client to the corresponding App. + This mapping exists so that + * you can have a different name for the OAuth client than for the app, and + * you can have multiple OAuth clients that belong to the same app. + """ + + __tablename__ = "oauthclient_app" + oauthclient_id = db.Column(String(length=64), primary_key=True) + app_id = db.Column(Integer, ForeignKey("app.id")) + + app = relationship("App") + + def __repr__(self): + return (f"oauthclient_id: {self.oauthclient_id}, app_id: {self.app_id}," + f" app: {self.app}") diff --git a/backend/migrations/versions/fc0892d07771_add_oauthclient_app_table.py b/backend/migrations/versions/fc0892d07771_add_oauthclient_app_table.py new file mode 100644 index 0000000000000000000000000000000000000000..be0ddf02201f097547d2a69346aa1479dabd976c --- /dev/null +++ b/backend/migrations/versions/fc0892d07771_add_oauthclient_app_table.py @@ -0,0 +1,39 @@ +"""Add oauthclient_app table + +Revision ID: fc0892d07771 +Revises: 3fa0c38ea1ac +Create Date: 2022-11-02 09:52:09.510764 + +""" +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import mysql + +# revision identifiers, used by Alembic. +revision = 'fc0892d07771' +down_revision = '3fa0c38ea1ac' +branch_labels = None +depends_on = None + + +def upgrade(): + oauthclient_app_table = op.create_table('oauthclient_app', + sa.Column('oauthclient_id', mysql.VARCHAR(length=64), nullable=False), + sa.Column('app_id', mysql.INTEGER(display_width=11), autoincrement=False, nullable=False), + sa.ForeignKeyConstraint(['app_id'], ['app.id'], name='oauthclient_app_fk_app_id'), + sa.PrimaryKeyConstraint('oauthclient_id'), + mysql_default_charset='utf8mb3', + mysql_engine='InnoDB' + ) + values = [ + {"oauthclient_id": "dashboard" , "app_id": 1}, + {"oauthclient_id": "wekan" , "app_id": 2}, + {"oauthclient_id": "wordpress" , "app_id": 3}, + {"oauthclient_id": "nextcloud" , "app_id": 4}, + {"oauthclient_id": "zulip" , "app_id": 5}, + {"oauthclient_id": "kube-prometheus-stack", "app_id": 6}, + ] + op.bulk_insert(oauthclient_app_table, values) + +def downgrade(): + op.drop_table('oauthclient_app') diff --git a/backend/web/login/login.py b/backend/web/login/login.py index 3601a00e1aea650423e87c18f1812cdedf3e77f0..acd61288c56146acb907648c3583949ee6ae991d 100644 --- a/backend/web/login/login.py +++ b/backend/web/login/login.py @@ -17,7 +17,7 @@ from database import db from helpers import KratosUser from config import * from web import web -from areas.apps import AppRole, App +from areas.apps import AppRole, App, OAuthClientApp from areas.roles import RoleService @@ -137,13 +137,13 @@ def login(): @web.route("/auth", methods=["GET", "POST"]) def auth(): - """Authorize an user for an application - If an application authenticated against the IdP (Idenitity Provider), if - there are no active session, the user is forwarded to the login page. + """Authorize a user for an application + If an application authenticated against the IdP (Identity Provider), if + there are no active sessions, the user is forwarded to the login page. This is the entry point for those authorization requests. The challenge as provided, is verified. If an active user is logged in, the request is accepted and the user is returned to the application. If the user is not - logged in yet, it redirects to the login page + logged in yet, it redirects to the login page. :param challenge: challenge as given by Hydra :return: redirect to login or application/idp """ @@ -212,9 +212,8 @@ def auth(): @web.route("/consent", methods=["GET", "POST"]) def consent(): """Get consent - For now, it just allows every user. Eventually this function should check - the roles and settings of a user and provide that information to the - application. + Rather than get consent from the end-user, this checks whether the user + should have access to the given app based on stackspin roles. :param consent_challenge: challenge as given by Hydra :return: redirect to login or render error """ @@ -242,11 +241,11 @@ def consent(): if isinstance(consent_client, str): consent_client = ast.literal_eval(consent_client) - app_id = consent_client.get("client_id") + client_id = consent_client.get("client_id") # False positive: pylint: disable=no-member kratos_id = consent_request.subject current_app.logger.info(f"Info: Found kratos_id {kratos_id}") - current_app.logger.info(f"Info: Found app_id {app_id}") + current_app.logger.info(f"Info: Found client_id {client_id}") except Exception as ex: current_app.logger.error( @@ -255,7 +254,7 @@ def consent(): current_app.logger.error(f"Error: {ex}") current_app.logger.error(f"Client: {consent_request.client}") current_app.logger.error(f"Subject: {consent_request.subject}") - abort(501, description="Internal error occured") + abort(501, description="Internal error occurred") # Get the related user object current_app.logger.info(f"Info: Getting user from admin {kratos_id}") @@ -277,10 +276,10 @@ def consent(): # If the user is dashboard admin admin is for all if role_object is not None and role_object.role_id == ADMIN_ROLE_ID: current_app.logger.info(f"Info: User has admin dashboard role") - current_app.logger.info(f"Providing consent to {app_id} for {kratos_id}") - current_app.logger.info(f"{kratos_id} was granted admin access to {app_id}") + current_app.logger.info(f"Providing consent to {client_id} for {kratos_id}") + current_app.logger.info(f"{kratos_id} was granted admin access to {client_id}") # Get claims for this user, provided the current app - claims = user.get_claims(app_id, ['admin']) + claims = user.get_claims(None, ['admin']) return redirect( consent_request.accept( grant_scope=consent_request.requested_scope, @@ -289,43 +288,52 @@ def consent(): ) ) - # Get role on this app - app_obj = db.session.query(App).filter(App.slug == app_id).first() + # Resolve to which app the client_id belongs. + app_obj = db.session.query(OAuthClientApp).filter(OAuthClientApp.oauthclient_id == client_id).first().app + if not app_obj: + current_app.logger.error(f"Could not find app for client {client_id}") + return redirect( + consent_request.reject( + error="No access", + error_description="The user has no access for app", + error_hint="Contact your administrator", + status_code=401, + ) + ) # Default access level roles = [] - if app_obj: - role_object = ( - db.session.query(AppRole) - .filter(AppRole.app_id == app_obj.id) - .filter(AppRole.user_id == user.uuid) - .first() - ) - # Role ID 3 is always "No access" due to migration b514cca2d47b - if role_object is None or role_object.role_id is None or role_object.role_id == NO_ACCESS_ROLE_ID: - # If there is no role in app_roles or the role_id for an app is null user has no permissions - current_app.logger.error(f"User has no access for: {app_obj.name}") - return redirect( - consent_request.reject( - error="No access", - error_description="The user has no access for app", - error_hint="Contact your administrator", - status_code=401, - ) + role_object = ( + db.session.query(AppRole) + .filter(AppRole.app_id == app_obj.id) + .filter(AppRole.user_id == user.uuid) + .first() + ) + # Role ID 3 is always "No access" due to migration b514cca2d47b + if role_object is None or role_object.role_id is None or role_object.role_id == NO_ACCESS_ROLE_ID: + # If there is no role in app_roles or the role_id for an app is null user has no permissions + current_app.logger.error(f"User has no access for: {app_obj.name}") + return redirect( + consent_request.reject( + error="No access", + error_description="The user has no access for app", + error_hint="Contact your administrator", + status_code=401, ) - else: - roles.append(role_object.role.name) + ) + else: + roles.append(role_object.role.name) current_app.logger.info(f"Using '{roles}' when applying consent for {kratos_id}") # Get claims for this user, provided the current app - claims = user.get_claims(app_id, roles) + claims = user.get_claims(None, roles) # pylint: disable=fixme # TODO: Need to implement checking claims here, once the backend for that is # developed - current_app.logger.info(f"Providing consent to {app_id} for {kratos_id}") - current_app.logger.info(f"{kratos_id} was granted access to {app_id}") + current_app.logger.info(f"Providing consent to {client_id} for {kratos_id}") + current_app.logger.info(f"{kratos_id} was granted access to {client_id}") # False positive: pylint: disable=no-member return redirect(