Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OIDC dynamic_registrations_file should have a relative path #4454

Closed
zzorba opened this issue Dec 23, 2024 · 3 comments
Closed

OIDC dynamic_registrations_file should have a relative path #4454

zzorba opened this issue Dec 23, 2024 · 3 comments

Comments

@zzorba
Copy link
Contributor

zzorba commented Dec 23, 2024

The current OIDC config for MAS has a dynamic_registrations_file that requires an absolute path directory.

This is generally fine, however it presents some challenges when working with the iOS simulator -- which has the following quirk when used with react-native.

  1. Every app has its own session directory for storing app specific user data.
  2. On the simulator, this path will change with every iOS build. However, the system makes sure to copy data from the old path to the new path when it does this rename.
  3. But the files will no longer be readable at the old path.
  4. This behavior DOES not appear to happen on real devices, it is a simulator quirk.

What you are supposed to do as an app developer, is store a path 'relative' to your app directory, and use the dynamic session path in combination with your stored relative path. Practically speaking, this means that every time you recompile the app, you get logged out and have to re-auth your OIDC system.

For the existing session_directory logic in non-OIDC flows this is possible to do the above because the session_directory is stored external to the matrix-rust-sdk and can be 'patched' on read. But it appears that the dynamic_registrations_file is persisted at login time.

I'm happy to make a PR to address this, but I'm curious if it would be accepted and could use a pointer as to where the OIDC config is being persisted, I couldn't seem to track down how that was happening.

@pixlwave
Copy link
Member

pixlwave commented Jan 6, 2025

Hey @zzorba, thanks for opening this issue. I'm not 100% sure this change makes sense. Starting with your last question about where the OIDC configuration is persisted:

The client metadata is held within the Session::oidc_data string (along with a client ID returned from the server), but neither the static_registrations nor the dynamic_registrations_file properties are stored. They are both used directly within Client::url_for_oidc and then forgotten, so you can safely construct this value before calling that and not worry about future directory changes.

If you were to pass a relative path for the registrations file, where would you expect the SDK would choose for the path to be relative to? The path is currently an absolute path as

  • The SDK can't use the data_path from ClientBuilder::session_paths here as the file needs to live outside of that in order to be shared across all accounts that are created by this client (past, present and future).
    • Maybe on this point it's worth double checking that you're not passing your user data directory directly to ClientBuilder::session_paths? The assumption is that you would use a new subdirectory within the user data directory every time you login to an account.
  • The SDK can't decide where to read this from by itself as this would be a platform specific choice (and even then, if you take iOS as an example it wouldn't know if the app wanted to use the applicationSupportDirectory/documentsDirectory or a shared App Group container).

@zzorba
Copy link
Contributor Author

zzorba commented Jan 6, 2025

Thank you for the response.

I've been trying to dig in more to understand why the device verification data is being lost between sessions when I sign in with OIDC, but not when I used to do a traditional login.

The symptom I'm seeing appears to be happening on app updates, which on iOS can be caused because the absolute path for app specific data changes between builds, but the underlying data is seamlessly moved from old to new directory by the OS. But I was already accounting for this with session data, and as you detailed here the OIDC files are not persisted (and their paths don't appear to be saved either).

I'm going to keep digging to figure out where the data might be going.

@pixlwave
Copy link
Member

pixlwave commented Jan 6, 2025

Ah ok, sorry to hear working with React isn't smooth on iOS (we haven't seen anything like you've described with Element X iOS as far as I'm aware). I'm going to close this issue as it doesn't sound like we need it any more.

@pixlwave pixlwave closed this as not planned Won't fix, can't repro, duplicate, stale Jan 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants