Clarify that authorize can be given just a token #186
Conversation
|
Thanks, LGTM. |
As in proposed RSA10e spec clarification in PR ably/docs#186
| @@ -1057,6 +1058,8 @@ h4. AuthOptions | |||
| * @(AO1)@ A class providing configurable authentication options used when authenticating or issuing tokens explicitly. These options are used when invoking @Auth#authorize@, @Auth#requestToken@, @Auth#createTokenRequest@ and @Auth#authorize@ | |||
| * @(AO2)@ The attributes of @AuthOptions@ consist of: | |||
| ** @(AO2a)@ @key@ string - Full Ably key string, as obtained from dashboard, used when signing token requests locally | |||
| ** @(AO2h)@ @token@ string - An authentication token string issued for this application | |||
| ** @(AO2i)@ @tokenDetails@ @TokenDetails@ - An authentication token issued for this application | |||
There was a problem hiding this comment.
An authentication token and associated details issued for this application ?
There was a problem hiding this comment.
Why and associated details?
There was a problem hiding this comment.
I mean just saying "an authentication token" might be understood to be the token string, whereas here we're talking about a TokenDetails, ie including expiry and other attributes.
There was a problem hiding this comment.
Added 0203f19 to clarify tokenDetails
As in proposed RSA10e spec clarification in PR ably/docs#186
As in proposed RSA10e spec clarification in PR ably/docs#186
As in proposed RSA10e spec clarification in PR ably/docs#186
As in proposed RSA10e spec clarification in PR ably/docs#186
|
Addressed comment, believe this is ready for merging |
|
I don't specifically think there is anything wrong with this proposal, but I am wondering why we are introducing this complexity. Under what conditions will people provide a token value and a means to obtain a token in reality? Also, should we even encourage people to provide tokens directly as part of AuthOptions when in most cases this will be wrong? Is there a strong enough use case to warrant this additional complexity versus simply rejecting authorize requests where customers explicitly provide a token? The only situation I can think is that perhaps a client receives a message (as a normal message or via another means) notifying it it should get a new token, and instead of just calling authorize they instead get the token and pass it to authorize. In that case though, why don't we rather encourage people to just call |
| @@ -107,6 +107,7 @@ h3(#rest-auth). Auth | |||
| *** @(RSA4c1)@An @ErrorInfo@ with @code@ @80019@ and description of the underlying failure should be emitted with the state change, in the @errorReason@ and/or in the callback as appropriate | |||
| *** @(RSA4c2)@If the connection is @CONNECTING@, then the connection attempt should be treated as unsuccessful, and as such the connection should transition to the @DISCONNECTED@ or @SUSPENDED@ state as defined in "RTN14":#RTN14 and "RTN15":#RTN15 | |||
| *** @(RSA4c3)@If the connection is @CONNECTED@, then the connection should remain @CONNECTED@ | |||
| ** @(RSA4d)@ If multiple @authOptions@ are used to initialize the library, the preference ordering among them is identical to @Auth#authorize@, defined in @RSA10e@ | |||
There was a problem hiding this comment.
I think "multiple authOptions" is confusing. What's an authOption? The formatting seems to indicate it's something formal in the code, like a specified field or type.
| @@ -159,7 +160,7 @@ h3(#rest-auth). Auth | |||
| ** @(RSA10j)@ Method signature is @authorize(TokenParams, AuthOptions)@. @TokenParams@ and @AuthOptions@ are optional. When the arguments are present, even if empty, the @TokenParams@ and @AuthOptions@ supersede any previously client library configured @TokenParams@ and @AuthOptions@. For example, if a client is initialised with @TokenParams#ttl@ configured with a custom value, and a @TokenParams@ object is passed in as an argument to @#authorize@ with a @null@ or missing value for @ttl@, then the @ttl@ used for every subsequent authorization will be @null@ | |||
| ** @(RSA10b)@ Supports all @AuthOptions@ and @TokenParams@ in the function arguments | |||
| ** @(RSA10k)@ If the @AuthOption@ argument's @queryTime@ attribute is true, it will obtain the server time once and persist the offset from the local clock. All future token requests generated directly or indirectly via a call to @authorize@ will not obtain the server time, but instead use the local clock offset to calculate the server time. The client library itself MAY internally discard the cached local clock offset in situations in which it may have been invalidated, such as if there is a local change to the date, time, or timezone, of the client device. For clarity however, there is no requirement for this cache invalidation to be available to consumers of the client library API. | |||
| ** @(RSA10e)@ Adheres to the implementation of @requestToken@ when issuing new tokens | |||
| ** @(RSA10e)@ If the @authOptions@ contains a way of obtaining a token (an @authCallback@, @authUrl@, or @key@), that should be used to obtain a new token, as per @requestToken@ (@RSA8@). If it contains a token (@token@ or @tokenDetails@), that should be used as-is. If it contains both a token and a way of obtaining a token, the token should be used, with the way of obtaining a token being stored per @RSA10g@ for when the token expires. (Ordering of preference within those groups is not defined and is left up to individual implementations) | |||
There was a problem hiding this comment.
Nitpick: either "authOptions" or "the AuthOptions". (I prefer the latter since we don't name the arguments in RSA10j, just the types.)
I think this proposal reduces complexity, and instead making a special case out of it like causing Plus, if we give them liberty to provide a token on initialization I don't see why we should restrict it afterwards. We ask for |
Ok, but let's push this into |
As in proposed RSA10e spec clarification in PR ably/docs#186
As in proposed RSA10e spec clarification in PR ably/docs#186
|
Can this be rebased onto |
As in proposed RSA10e spec clarification in PR ably/docs#186
As in proposed RSA10e spec clarification in PR ably/docs#186
|
@SimonWoolf this now needs to be rebased on |
| @@ -107,6 +107,7 @@ h3(#rest-auth). Auth | |||
| *** @(RSA4c1)@An @ErrorInfo@ with @code@ @80019@ and description of the underlying failure should be emitted with the state change, in the @errorReason@ and/or in the callback as appropriate | |||
| *** @(RSA4c2)@If the connection is @CONNECTING@, then the connection attempt should be treated as unsuccessful, and as such the connection should transition to the @DISCONNECTED@ or @SUSPENDED@ state as defined in "RTN14":#RTN14 and "RTN15":#RTN15 | |||
| *** @(RSA4c3)@If the connection is @CONNECTED@, then the connection should remain @CONNECTED@ | |||
| ** @(RSA4d)@ If multiple @authOptions@ are used to initialize the library, the preference ordering among them is identical to @Auth#authorize@, defined in @RSA10e@ | |||
| @@ -159,7 +160,7 @@ h3(#rest-auth). Auth | |||
| ** @(RSA10j)@ Method signature is @authorize(TokenParams, AuthOptions)@. @TokenParams@ and @AuthOptions@ are optional. When the arguments are present, even if empty, the @TokenParams@ and @AuthOptions@ supersede any previously client library configured @TokenParams@ and @AuthOptions@. For example, if a client is initialised with @TokenParams#ttl@ configured with a custom value, and a @TokenParams@ object is passed in as an argument to @#authorize@ with a @null@ or missing value for @ttl@, then the @ttl@ used for every subsequent authorization will be @null@ | |||
| ** @(RSA10b)@ Supports all @AuthOptions@ and @TokenParams@ in the function arguments | |||
| ** @(RSA10k)@ If the @AuthOption@ argument's @queryTime@ attribute is true, it will obtain the server time once and persist the offset from the local clock. All future token requests generated directly or indirectly via a call to @authorize@ will not obtain the server time, but instead use the local clock offset to calculate the server time. The client library itself MAY internally discard the cached local clock offset in situations in which it may have been invalidated, such as if there is a local change to the date, time, or timezone, of the client device. For clarity however, there is no requirement for this cache invalidation to be available to consumers of the client library API. | |||
| ** @(RSA10e)@ Adheres to the implementation of @requestToken@ when issuing new tokens | |||
| ** @(RSA10e)@ If the @authOptions@ contains a way of obtaining a token (an @authCallback@, @authUrl@, or @key@), that should be used to obtain a new token, as per @requestToken@ (@RSA8@). If it contains a token (@token@ or @tokenDetails@), that should be used as-is. If it contains both a token and a way of obtaining a token, the token should be used, with the way of obtaining a token being stored per @RSA10g@ for when the token expires. (Ordering of preference within those groups is not defined and is left up to individual implementations) | |||
|
Merged into master-1-1 as 6ce7fba |
tpr was pointing out in #client-libraries today that the auth#authorize spec doesn't make clear whether you can give it a token and it should use that -- https://ably-real-time.slack.com/archives/client-libraries/p1478012497000052. Proposed clarification