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

chore(toolkit): update mobileUriSchemeProtocolRegEx #5843

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ImSingee
Copy link
Contributor

@ImSingee ImSingee commented May 10, 2024

Summary

There's no requirement that the mobile schema must be xxx.my://; a simple myapp:// should be okay.

Testing

Before:

image

After:
image

Checklist

  • .changeset
  • unit tests
  • integration tests
  • necessary TSDoc comments

@github-actions github-actions bot added the chore Hmm... label May 10, 2024
Copy link

github-actions bot commented May 10, 2024

COMPARE TO master

Total Size Diff 📉 -27 Bytes

Diff by File
Name Diff
packages/toolkit/core-kit/src/regex.ts 📉 -12 Bytes
packages/toolkit/core-kit/src/utils/url.test.ts 📉 -15 Bytes

@ImSingee ImSingee force-pushed the patch-1 branch 2 times, most recently from 9bb58f0 to 4e27dc1 Compare May 10, 2024 08:07
@ImSingee ImSingee changed the title chore(regex): update mobileUriSchemeProtocolRegEx chore(toolkit): update mobileUriSchemeProtocolRegEx May 10, 2024
@charIeszhao
Copy link
Member

charIeszhao commented May 16, 2024

Well, unfortunately our underlying oidc provider library requires the . to be presented in the custom scheme for native apps.
https://github.com/panva/node-oidc-provider/blob/main/lib/helpers/client_schema.js#L566-L568

@ImSingee
Copy link
Contributor Author

@charIeszhao Sadly to see that. I really can't understand the reason that URL scheme of the native app must be xx.xx://. There are tons of apps that have a single xx:// URL scheme.

@charIeszhao
Copy link
Member

@charIeszhao Sadly to see that. I really can't understand the reason that URL scheme of the native app must be xx.xx://. There are tons of apps that have a single xx:// URL scheme.

Well, I understand that in real life there are many apps that do not have a domain based scheme, however, it is clearly stated in the IETF standard that these schemes should not be used.

Here's the documentation for your reference: https://datatracker.ietf.org/doc/html/draft-ietf-oauth-native-apps-06#section-7.1.1

@ImSingee
Copy link
Contributor Author

@charIeszhao Another similar problem: I'm developing an app with Expo, which supports both React Native and React Web. But I can't configure the https scheme to the native app, and I can't configure the xx.xx schema to the web app. So, it seems there's no way to use a single appId for both platforms. Is it possible to support apps like this in Logto?

@charIeszhao
Copy link
Member

charIeszhao commented May 29, 2024

@charIeszhao Another similar problem: I'm developing an app with Expo, which supports both React Native and React Web. But I can't configure the https scheme to the native app, and I can't configure the xx.xx schema to the web app. So, it seems there's no way to use a single appId for both platforms. Is it possible to support apps like this in Logto?

A possible way I can think of is to set the scheme during building process, maybe. So you can still use the same code base, but when building the app for different platforms, you can have different uri schemes. Or do you have like a env flag in your code, like isMobileApp or isWebApp?

There might still be other better practices

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

None yet

2 participants