Conversation
mullermp
force-pushed
the
endpoints-auth
branch
from
4aaa535 to
c885c23
Compare
jterapin
left a comment
jterapin
left a comment
There was a problem hiding this comment.
LGTM but there appears to be some RBS failure - FYI.
| class ApiKeySigner | ||
| def initialize(options = {}) | ||
| @name = options[:name] | ||
| @in = options[:in] | ||
| @scheme = options[:scheme] | ||
| end |
There was a problem hiding this comment.
Are these intended to be public interfaces - if so, we should include more documentation about init options? Here, other signers and auth scheme.
There was a problem hiding this comment.
I intended them to be API private for now. I'm also not sure yet so I'm going to wait on documentation.
richardwang1124
left a comment
richardwang1124
left a comment
There was a problem hiding this comment.
Looks good overall to me. There's a few failing RBS tests.
| end | ||
|
|
||
| def resolve_without_endpoint_auth(config, auth_parameters) | ||
| auth_options = config.auth_resolver.resolve(auth_parameters) |
There was a problem hiding this comment.
If I'm understanding this correctly, endpoint auth completely overrides the modeled auths?
There was a problem hiding this comment.
Yeah. I think we should do it that way. It's less complex and I think endpoint auth is guaranteed to work.
|
|
||
| properties[key] = value | ||
| end | ||
| normalized_endpoint_schemes << { scheme_id: normalized_scheme_id, signer_properties: properties } |
There was a problem hiding this comment.
I'm assuming we'll need to make changes to sigv4 auth in V4 as well following these changes. Have you tested with V4?
There was a problem hiding this comment.
Yeah, I tested with v4 and I opened a separate PR.
3 participants
Resolves auth using an auth module instead of entirely in the resolve auth plugin. This is necessary so that presigners and other auth related utilities can resolve auth. The auth is now once again an auth scheme and has an attached signer and identity provider, so that signing can be done.
Also implements auth scheme preference config.