-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
fix(mobile): explicit launcher uri for oauth #10832
base: main
Are you sure you want to change the base?
Conversation
🤔 I've got some unexpected localization edits in there. Maybe upstream history changed. I'll reconcile those. |
d91af9b
to
6651132
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've reverted the unexpected changes.
📖 Documentation deployed to pr-10832.preview.immich.app |
FYI, in the future, for mobile translation, you only need to modify the |
You have some prettier failures, if you could fix those then we can look at this PR |
6651132
to
ded8b29
Compare
I get the expected result with the APK build on my Pixel 6 Pro w/ Android 14. Resolves #2932
To test, I used an Auth0 generic web app with allowed callback
app.immich:///
, which represents a request for the root/
resource fromapp.immich
with a standard://
separator between scheme and resource.It's my first contribution so I'll be orienting myself to testing norms after I see what fires off with this submission.
Happy to iterate on this. It will be a breaking change for anyone using mobile OAuth. The fix for them is to modify their IdP to allow callback
app.immich:///
. If they're using the mobile callback override URI then it might go unnoticed ifapp.immich:/
was successfully launching the app and concluding the login flow. It's not clear to me whether this is currently broken for everyone.