Skip to content
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

Feature request: support @httpApiKeyAuth with scheme #453

Closed
glb opened this issue Oct 15, 2021 · 12 comments · Fixed by #473
Closed

Feature request: support @httpApiKeyAuth with scheme #453

glb opened this issue Oct 15, 2021 · 12 comments · Fixed by #473

Comments

@glb
Copy link
Contributor

glb commented Oct 15, 2021

In response to smithy-lang/smithy#872, smithy-lang/smithy#893 added support for describing services that require a custom scheme value in the Authorization header; for example:

    @httpApiKeyAuth(scheme: "ApiKey", name: "Authorization", in: "header")
     service WeatherService {
         version: "2017-02-11",
     }

if the Authorization header is supposed to look like Authorization: ApiKey {value}. The folks there said that opening an issue here to request support would be the best next step to building an SDK that can interact with a service like this.

Searching through this repository I wasn't able to find any references to auth or authorization, so I'm not sure where to start. Opening this issue to start the conversation.

@glb
Copy link
Contributor Author

glb commented Oct 22, 2021

@mtdowling thanks for your help with the original change in Smithy and the recommendation to take next steps here. Do you know who I should ping to start the conversation?

@JordonPhillips
Copy link
Contributor

Right now the only auth that's implemented for smithy-typescript is AWS's sigv4 auth: https://github.com/aws/aws-sdk-js-v3/blob/main/codegen/smithy-aws-typescript-codegen/src/main/java/software/amazon/smithy/aws/typescript/codegen/AddAwsAuthPlugin.java

To implement api key auth, you'd need to write a plugin to handle it much like that. I'm not sure when we'd be getting around to doing this, but if you want to tack a shot at it then we'd be happy to review.

@glb
Copy link
Contributor Author

glb commented Nov 23, 2021

Thanks for the pointer @JordonPhillips . I'll try to figure it out, will quite likely have questions.

@glb
Copy link
Contributor Author

glb commented Dec 3, 2021

OK @JordonPhillips @mtdowling, I'm back with the questions I promised 🙂:

  • It looks like the plugin needs to add a field to the config interface (the API key) and then create / inject a middleware handler into the client. I think that means I need to implement addConfigInterfaceFields and getClientPlugins. Is that right-ish? The first one seems pretty straightforward (and seems to have worked!), the second is a bit more opaque. I'll try to dig into what's going on there but will likely have another layer of questions when I get into that.
  • My code builds, but next how do I register my new plugin so that it gets invoked when doing codegen? I wonder if adding it to src/main/resources/META-INF/services/software.amazon.smithy.typescript.codegen.integration.TypeScriptIntegration will do the trick, looks possible...
  • This may seem like a super-basic question, but I haven't touched Java in >4 years, so: how do I do a real test? I've got a model file that I think has the right content, so do I just run gradle publishToMavenLocal in this repo and then do a gradle build in my model repo? apparently yes! and holy guacamole it worked! at least I see a generated client with the additional config attribute! 🎉

I'll keep digging but thought I'd get these questions out there.

@glb
Copy link
Contributor Author

glb commented Dec 3, 2021

Update: I think I've gotten very close to where I want. I don't particularly love the idea of having to publish a separate middleware module to set a header (though I have seen that there's already one in the AWS SDK to set the Host header), so am going to see what I can figure out so that getHttpApiKeyAuthPlugin can be an inline function. Is that a step too far?

commands/ListThingsCommand.ts
25:import { getHttpApiKeyAuthPlugin } from "fake-hackery";
49:    this.middlewareStack.use(getHttpApiKeyAuthPlugin(configuration));

I haven't yet figured out how to extract the scheme value from the trait and where the right place to put it will be.

@glb
Copy link
Contributor Author

glb commented Dec 4, 2021

Update: I've figured out how to use writeAdditionalFiles to write the middleware module, and I think I've got everything relevant to the plugin working. No idea yet if the actual middleware works or if the generated typescript code is good.

Two parallel paths of investigation next:

  • Learn enough TypeScript to try to use the generated client.

  • Figure out why I'm getting this error and how to fix it because the generated client doesn't actually know how to serialize requests:

    Unable to find a protocol generator for example.com#Example: The example.com#Example service supports the following unsupported protocols [aws.protocols#restJson1]. The following protocol generators were found on the class path: []
    

    Turns out I was missing smithy-aws-typescript-codegen as a dependency in my build.gradle.kts file. Seems to have serialization now! 🎉 ?

@glb
Copy link
Contributor Author

glb commented Dec 4, 2021

Next layer of the onion: I'm trying to use

   .pluginFunction(Symbol.builder().namespace("./middleware/HttpApiKeyAuth", "/")
                                   .name("getHttpApiKeyAuthPlugin")
                                   .build())

but that's translating into ... = require("../../middleware/HttpApiKeyAuth"); in operations and ... = require("../middleware/HttpApiKeyAuth") in the client; these don't work but taking out the extra .. manually after codegen does. Need to figure out what's going on in the codegen there and how to fix it.

Had a brief hiccup around endpoint but figured out that I can just pass it in to the client constructor like in the normal AWS SDK and it's ... actually connecting and making a request!

Also need to figure out how to change these; hopefully it's just something I shouldn't have in my model or something I'm missing:

x-amz-user-agent: aws-sdk-js/0.0.1
user-agent: aws-sdk-js/0.0.1 os/darwin/21.1.0 lang/js md/nodejs/14.17.5
amz-sdk-invocation-id: aa728ff4-e7f2-4b48-9923-20d0b966bf79
amz-sdk-request: attempt=1; max=3

So close!

@glb
Copy link
Contributor Author

glb commented Dec 4, 2021

Figured out the correct magic by looking at other places where things were being imported:

                .pluginFunction(Symbol.builder()
                    .namespace("./" + CodegenUtils.SOURCE_FOLDER + "/middleware/HttpApiKeyAuth", "/")
                    .name("getHttpApiKeyAuthPlugin")
                    .build())

makes the imports relativize properly.

The AWS SDK headers are an unrelated problem that I'm choosing to ignore for now.

Next step: tests. Will look at AddAwsAuthPluginTest.java for inspiration.

@glb
Copy link
Contributor Author

glb commented Dec 4, 2021

Initial tests for the Java code are done. Need to learn how to write tests for the middleware. Back to AddAwsAuthPlugin.java:223 to see how tests get added to the client and sts-client-defaultRoleAssumers.spec.ts for examples!

@glb
Copy link
Contributor Author

glb commented Dec 5, 2021

So sts-client-defaultRoleAssumers.spec.ts wasn't a great source of applicable testing examples but middleware-host-header/src/index.spec.ts really was. Tests seem to do the right thing now.

@glb
Copy link
Contributor Author

glb commented Dec 6, 2021

I intend to create a draft PR in the morning. Anything I should keep in mind other than what's in CONTRIBUTING.md and looking at existing PRs for examples?

@glb
Copy link
Contributor Author

glb commented Dec 7, 2021

Draft PR created for the next phase of discussion. Happy to mark this as "ready for review" if that's your workflow preference. The PR is awaiting maintainer approval to run the GitHub workflow.

image

Looking forward to your comments!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants