Skip to content

Conversation

LukasHirt
Copy link
Collaborator

@LukasHirt LukasHirt commented Aug 18, 2025

Description

We've added a capability to check if MFA is enabled. If the capability is enabled, we will require MFA when accessing the admin settings page.

Motivation and Context

Admin settings can be made available only to users with configured 2FA.

How Has This Been Tested?

For easier testing, there is a Keycloak example with acr configured introduced in owncloud/ocis#11592 and currently oCIS needs to be run with owncloud/ocis#11603 and latest Reva.

  • test environment: macos, chrome
  • test case 1: open admin settings with only "regular" acr
  • test case 2: open admin settings with "advanced" acr

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests
  • Documentation
  • Maintenance (e.g. dependency updates or tooling)

@LukasHirt LukasHirt self-assigned this Aug 18, 2025
@LukasHirt LukasHirt added the Category:Enhancement Add new functionality label Aug 18, 2025

This comment was marked as outdated.

@jvillafanez
Copy link
Member

A couple of important things:

  • The "acr" scope might not be available in the IDP. Requesting it under those conditions might break the authentication flow and prevent the user from login.
    • You should be able to test with those conditions using the keycloak deployment example -> oCIS realm -> client scopes -> remove the acr scope.
  • The "regular" and "advanced" values might change. Hardcoding them will cause problems.
    • You might get some value from the opeind-configuration endpoint, "acr_values_supported" keys. However, not all values might be present, and the order of the authentication level isn't guaranteed.
    • Relying on a default value from keycloak (without sending any value ourselves) might work, but it might cause problems later during reauthentication (needs further research)

owncloud/ocis#11603 has some information about the expected behavior of the clients regarding the feature detection.
Basically, if there is a 401 error code AND the headers X-OCIS-OIDC-Requires-Type: Acr and X-OCIS-OIDC-Requires-Data: acr=advanced are present in the response, the client should trigger the step up auth flow. The authentication level that the client should request is also part of the X-OCIS-OIDC-Requires-Data header (no need to hardcode it).
Strictly speaking, clients should NOT check for specific paths, but rely on the above conditions in order to trigger the step up auth flow.

@LukasHirt
Copy link
Collaborator Author

@jvillafanez the values are hardcoded for now just for testing. They will be replaced once the capabilities are implemented as per the ticket:

Note: There will be a capability for that. Capability contains: Enabled/Disabled, Claim name, Authentication level names.

@LukasHirt
Copy link
Collaborator Author

LukasHirt commented Aug 20, 2025

Strictly speaking, clients should NOT check for specific paths

Depending on the headers might prove very complex in the FE because we might have different situations where e.g. there is an action in progress which needs to be preserved until the user goes through OTP, etc. Since it's not only about handling a common redirect and nothing more, doing a generic handler depending on the header would then require bloated catch-all solution for such specific situations + we might have cases where we need to display things differently within the UI which would not be possible if we would have to first do a request...

That being said, we should of course have at least a simple fallback implemented which does the redirect in case we encounter those headers so that we catch any potential error or missing implementation.

One more thing is that we would temporarily show the UI of admin in a loading state if we would wait for the response.

@LukasHirt LukasHirt force-pushed the feat/add-acr-verification branch from a15c1c5 to 316cde3 Compare September 18, 2025 14:37
We've added a capability to check if MFA is enabled.
If the capability is enabled, we will require MFA when accessing
the admin settings page.
@LukasHirt LukasHirt force-pushed the feat/add-acr-verification branch from 006af87 to 77be8c5 Compare September 18, 2025 15:40
@LukasHirt LukasHirt changed the title feat: [OCISDEV-249] add acr verification to admin settings feat: [OCISDEV-249] add MFA capability Sep 18, 2025
@LukasHirt LukasHirt marked this pull request as ready for review September 18, 2025 15:41
@LukasHirt LukasHirt requested a review from mzner September 18, 2025 15:41
Copy link

@LukasHirt LukasHirt merged commit f1ea145 into master Sep 19, 2025
4 checks passed
@LukasHirt LukasHirt deleted the feat/add-acr-verification branch September 19, 2025 07:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category:Enhancement Add new functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants