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

Implement Pre-Authenticated URL #193

Merged
merged 24 commits into from
Jul 22, 2024
Merged

Conversation

tung2744
Copy link
Contributor

@tung2744 tung2744 commented Jul 8, 2024

ref DEV-1407

Test steps are same as authgear/authgear-sdk-js#306

@tung2744 tung2744 changed the title Implement app-initiated-sso-to-web Implement Pre-Authenticated URL Jul 11, 2024
@tung2744
Copy link
Contributor Author

Finished renaming.

@tung2744 tung2744 mentioned this pull request Jul 17, 2024
sdk/src/main/java/com/oursky/authgear/ActorTokenType.kt Outdated Show resolved Hide resolved
sdk/src/main/java/com/oursky/authgear/SubjectTokenType.kt Outdated Show resolved Hide resolved
sdk/src/main/java/com/oursky/authgear/SharedStorage.kt Outdated Show resolved Hide resolved
@@ -0,0 +1,12 @@
package com.oursky.authgear

sealed class AppInitiatedSSOToWebNotAllowedException : AuthgearException {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not use sealed class in public API. I do not know how is it translated to Java. Just use normal class hierarchy to represent these exceptions.

Copy link
Contributor Author

@tung2744 tung2744 Jul 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://medium.com/asos-techblog/kotlin-classes-in-java-world-365323843e6e

It will be something like

abstract class PreAuthenticatedURLNotAllowedException {
  static final class InsufficientScopeException extends PreAuthenticatedURLNotAllowedException {
  }
  static final class IDTokenNotFoundException extends PreAuthenticatedURLNotAllowedException {
  }
  static final class DeviceSecretNotFoundException extends PreAuthenticatedURLNotAllowedException {
  }
}

I've added a commit to ensure it can be imported in java.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But exception as a inner class in uncommon. I still prefer we use regular classes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@louischan-oursky Added a commit to change it to regular classes.

sdk/src/main/java/com/oursky/authgear/AuthgearCore.kt Outdated Show resolved Hide resolved
@louischan-oursky
Copy link
Contributor

@tung2744 Please ping me when this PR is ready for review again.

@tung2744
Copy link
Contributor Author

@louischan-oursky Updated, thanks!

@louischan-oursky louischan-oursky self-requested a review July 22, 2024 05:36
@louischan-oursky louischan-oursky merged commit e2669cb into authgear:main Jul 22, 2024
3 checks passed
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

Successfully merging this pull request may close these issues.

2 participants