-
Notifications
You must be signed in to change notification settings - Fork 1
SDKS-3971 Journey Module Design and Prototype #51
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
base: develop
Are you sure you want to change the base?
Conversation
val journey = Journey { | ||
timeout = 30 // Network request timeout in seconds (default: 30s) | ||
logger = Logger.STANDARD // Use the standard logger for Logcat output | ||
realm = "<realm_name>" // Specify the realm for authentication |
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.
Do we need the realmName
if the discovery endpoint was used?
In general, in the old SDK we had either discoveryEndpoint
or realm
, url
as mandatory fields
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.
The realm
is for the Journey module, the discoveryEndpoint
is under the OIDC
configuration for endpoint lookup only.
We need both for Journey with OIDC configured.
```kotlin | ||
var node = journey.start("myLogin") { | ||
forceAuth = true | ||
noSession = true |
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 like that! FYI the mustRun
server side configuration makes the forceAuth
flag a bit obsolete in some cases. As a general note we need to test with that in the mix as well
suspend fun start( | ||
request: Request, | ||
context: SharedContext = SharedContext(mutableMapOf()) | ||
): Node = lock.withLock { |
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.
Let's discuss the thread management you do on Android at some point. It would be good to be aligned between this and iOS. There are some use cases we need to cover, not so much Journey related, but mostly on the OIDC module
* Configuration class for CustomParameter. | ||
* Allows adding custom parameter to be injected into requests. | ||
*/ | ||
@PingDsl |
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.
What is this?
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.
This is specific for Kotlin language, it is used for managing the DSL, which is for json like configuration.
Journey {
logger = Logger.STANDARD
serverUrl = "https://openam-sdks.forgeblocks.com/am"
realm = "alpha"
cookie = "5421aeddf91aa20"
// Oidc as module
module(Oidc) {
clientId = "AndroidTest"
discoveryEndpoint =
"https://openam-sdks.forgeblocks.com/am/oauth2/alpha/.well-known/openid-configuration"
scopes = mutableSetOf("openid", "email", "address", "profile", "phone")
redirectUri = "org.forgerock.demo:/oauth2redirect"
//storage = dataStore
}
fun Journey(block: JourneyOptions.() -> Unit = {}): Journey { | ||
val config = JourneyOptions() | ||
|
||
internal const val ACCEPT_API_VERSION = "Accept-API-Version" |
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 would suggest centralizing consts like these or the ones earlier in the file on a per module file
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.
Yes, will create a constant file for each module. But will handle it on actual implementation Stories.
@@ -21,11 +21,4 @@ internal data class SSOTokenImpl( | |||
override val value: String, | |||
override val successUrl: String, |
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.
We might need to add the raw response as well. If not mistaken there is a chance that the Journey sets a FailureURL as well, so we need that and also we had a case that they added custom key/values they needed to access. Providing the raw response future proofs it
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.
To be honest, I think the SSOToken should only contains the token, and it will be persisted to the Storage. Not sure we really need the successUrl
and realm
to be persisted.
When developer receive SuccessNode
or ErrorNode
, they have access to the return json, so they have access to the custom key/value (Set by Set Success Details Node
and Set Failure Details Node
) with the Node, but not the persisted token.
sharedContext[OIDC_CLIENT_CONFIG] = config | ||
config.init() | ||
} | ||
|
||
start { request -> | ||
|
||
// When user starting the flow again, revoke previous token if exists | ||
// Revoke the previous token if the user is starting the flow again | ||
journey.user()?.revoke() |
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.
Is that going to revoke if we start a new Journey? This is not correct as we need to support noSession
journeys and also self service journeys. Do we have a way that if you enable the OIDC module, to call a journey similar to how we used to call with FRSession.authenticate("Login")
?
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 think we may need to discuss this with the Team.
This will only revoke the AccessToken, but not the Session Token.
With noSession, the existing session will not be overridden, if the Session still valid, calling User.token() will return new AccessToken.
daa8764
to
4e26d26
Compare
1bdeb69
to
cc42033
Compare
cca6e57
to
4618988
Compare
ab3764d
to
c8454a2
Compare
JIRA Ticket
SDKS-3917
Description
Journey Module Implementation (SDKS-3917): Key Updates
Existing Module Enhancements
foundation/android
foundation/logger
STANDARD
logger now handles long messages more effectively by segmenting them in logcat.foundation/oidc
User
interface now features arefresh()
function for on-demand access token updates.foundation/orchestrate
FlowContext
.Sample App Updates
refresh
andrevoke
functionality via a long-press menu on the token screen.