Skip to content
This repository was archived by the owner on Aug 10, 2026. It is now read-only.

Make all TokenDetail fields apart from token optional - #1427

Closed
Morganamilo wants to merge 1 commit into
mainfrom
optional-tokendetails
Closed

Make all TokenDetail fields apart from token optional#1427
Morganamilo wants to merge 1 commit into
mainfrom
optional-tokendetails

Conversation

@Morganamilo

Copy link
Copy Markdown

NOTE TO REVIEWERS - please do not review PRs in the DRAFT state, as the PR may change substantially before it is ready to review. Thanks.

Description

RSA16b says:

If the library is provided with a token without the corresponding
TokenDetails, then this holds a TokenDetails instance in which only
the token attribute is populated with that token string

RSA16b says:
If the library is provided with a token without the corresponding
TokenDetails, then this holds a TokenDetails instance in which only
the token attribute is populated with that token string
@Morganamilo

Copy link
Copy Markdown
Author

Similarly, Auth#authorize and Auth#requestToken both return TokenDetails yet AuthCallback and AuthUrl permit returning just a token leading me to assume they must also return a TokenDetails with just token set.

@lmars

Copy link
Copy Markdown
Member

@Morganamilo having chatted with Paddy, I think if we are to make a change here, it should probably be to stop wrapping token strings in a TokenDetails object, have a return type that better communicates the fact that a token may either by a string literal or a TokenDetails object, and continue to expect these TokenDetails fields to be non-null.

In the Rust SDK, we have the Token enum which is returned form an auth callback, and that can either be a token request, token details, or token literal (inline with spec point RSA8d), so perhaps that (or something similar) needs to make its way into the IDL?

@Morganamilo

Copy link
Copy Markdown
Author

I didn't see this reply sorry. I actually changed the rust to be this:

pubstructTokenDetails{pubtoken:String,pubmetadata:Option<TokenMetadata>,}pubstructTokenMetadata{pubexpires:DateTime<Utc>,pubissued:DateTime<Utc>,pubcapability:String,pubclient_id:Option<String>,}

Maybe this would work better on the IDL.

@QuintinWillison

Copy link
Copy Markdown
Contributor

This work will be revisited once the features spec has been moved to its new home under ably/specification#1.

Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@Morganamilo@lmars@QuintinWillison@tbedford