add guidance on implementing identity&auth workflow#2906
add guidance on implementing identity&auth workflow#2906lucix-aws wants to merge 3 commits intosmithy-lang:mainfrom
Conversation
|
This pull request does not contain a staged changelog entry. To create one, use the Make sure that the description is appropriate for a changelog entry and that the proper feature type is used. See |
|
|
||
| ```java | ||
| public interface IdentityResolver<TIdentity extends Identity> { | ||
| TIdentity resolve(Object properties); |
There was a problem hiding this comment.
The identity resolver / signer property bags are not formally defined here so I've just left them with an Object placeholder. I think the typed property bag construct needs its own section in these guidance docs.
| // model the Signer as an interface that returns a copied, modified transport | ||
| // message instead. | ||
| public interface Signer<TIdentity extends Identity, TMessage extends Message> { | ||
| void sign(TIdentity identity, TMessage message, Object properties); |
There was a problem hiding this comment.
The original doc assumed HTTP, I know formally we want to speak in terms of transport agnosticism so I've parameterized the transport type.
| ```java | ||
| // in this example, the service supports some combination of | ||
| // smithy.api#httpBearerAuth and aws.auth#sigv4 | ||
| public MyServiceClientConfig defaultConfig() { |
There was a problem hiding this comment.
These types of code snippets seem useful to the reader but as we build out these docs we want to make sure we have a standard for stuff like this (i.e. showing how to bootstrap a default config).
| This may be implemented like the following: | ||
|
|
||
| ```java | ||
| public void resolveAuthScheme(OperationContext ctx) { |
There was a problem hiding this comment.
Again, including snippets like this seems helpful but we should be consistent. I elected to just model the examples as using this opaque / undescribed "operation context" input that has the stuff you'd expect on a request.
| ) {} | ||
| ``` | ||
|
|
||
| ## Order of Operations |
There was a problem hiding this comment.
I have issues with this as written tbh. It explains how to use all the parts but it feels incomplete. Perhaps referring to an "order of operations" section once we do that (like the original docs have) will make it make more sense.
|
|
||
| ## FAQ | ||
|
|
||
| ### Wow, this seems like a lot. Do I really need all of these abstractions just to decide how to set an Authorization header? |
There was a problem hiding this comment.
Probably controversial but this felt absolutely worth stating IMO. I think a lot of client designers will sort of stick to baking in a single transport type (probably HTTP). The language of the question is probably too colloquial ofc.
|
MISSING! An actual example of an AuthSchemeResolver. IMO that is perhaps worth including but I know we are focusing on the runtime here rather than what's code-generated. I think actually showing an example of one for a simple 2-3 operation service that does per-operation multiauth would be of value to the reader, though. |
JordonPhillips
left a comment
JordonPhillips
left a comment
There was a problem hiding this comment.
General feedback:
When referring to a class in text, it should be enclosed in an inline block with backticks (Foo). I started making comments for each, but there's quite a few.
I'll do another pass later after I've had a chance to think more about the interfaces themselves
| 1. Retrieve the IdentityResolver from the previously-resolved AuthScheme | ||
| (AuthScheme::identityResolver). Identity resolution | ||
| (IdentityResolver::resolve) is called with the identity properties sourced from | ||
| scheme resolution. | ||
| 1. Retrieve the Signer from the previously-resolved AuthScheme | ||
| (AuthScheme::signer). Request signing (Signer::sign) is called with the | ||
| merged signer properties from scheme and endpoint resolution. |
There was a problem hiding this comment.
These both mix passive and imperative, a consistent tense should be used
|
|
||
| ### What about operations with no authentication? | ||
|
|
||
| "Anonymous" (no authentication) is explicitly modeled via `@smithy.api#noAuth`. |
There was a problem hiding this comment.
This is what appears when you use the auth knowledge index, but it's a synthetic trait that isn't explicitly modeled. Anonymous auth is specified with an empty auth trait list (@auth([])) or as a consequence of using the optionalAuth trait.
Optional auth should also be discussed here.
Co-authored-by: Jordon Phillips <JordonPhillips@users.noreply.github.com>
lucix-aws
force-pushed
the
scg-identityauth
branch
from
c451d5b to
ea11820
Compare
2 participants
No description provided.