feat(api): add support for oauth2 token endpoint #376
Conversation
acwrenn
force-pushed
the
add_oauth2_token_flow
branch
2 times, most recently
from
bc4e4d5 to
5533218
Compare
acwrenn
force-pushed
the
add_oauth2_token_flow
branch
4 times, most recently
from
c364b4e to
493572b
Compare
src/server/server.ts
Outdated
| let rawBody = ""; | ||
| req.setEncoding("utf8"); | ||
| req.on("data", function (chunk) { | ||
| rawBody += chunk; | ||
| }); | ||
| req.on("end", function () { | ||
| const target = "GetToken"; | ||
| const route = router(target); | ||
| const auth = req.get("Authorization"); | ||
| if (auth && auth.startsWith("Basic ")) { | ||
| const sliced = auth.slice("Basic ".length); | ||
| const buff = new Buffer(sliced, "base64"); | ||
| const decoded = buff.toString("ascii"); | ||
| const creds = decoded.split(":"); | ||
| if (creds.length == 2) { | ||
| const id = creds[0]; | ||
| const secret = creds[1]; | ||
| rawBody += `&client_id=${id}`; | ||
| rawBody += `&client_secret=${secret}`; | ||
| } | ||
| } |
There was a problem hiding this comment.
This is a bit gnarly. 😄
I'm assuming the POST body is form encoded? I'd suggest you either:
- Configure the urlencoded body parser and that'll make
req.bodybe key-value pairs if the correctContent-Typeis sent on the request, then you'll have a proper object to deal with instead of passing strings around. - Or at least use the raw body parser which'll make
req.bodyinto a Buffer you can call toString on, instead of having to concat from a stream.
My preference would be for (1) and then also update the types of GetToken to something more specific too.
There was a problem hiding this comment.
Ok - I adjusted a little, lifted the URL parsing up to the top here, brought it into its own type, then injected the Authorization headers into the new request type instead of smashing it onto the string there.
I might have gone a little overboard with the type things, but I dont write a ton of typescript so got to have a little fun with it :)
|
I've left a comment on the implementation, but otherwise I think this looks ok. I don't mind it being a bit hacky for these non-API routes, at least while there's only a couple of them. Let me know what you think. The yarn and docker issues are odd though. There's a |
Cognito supports more features than just its AWS-API calls demonstrate. This service fake simulates just those AWS APIs - except for .well-known/jwks.json. This PR starts to add support for the token endpoint outlined here: https://docs.aws.amazon.com/cognito/latest/developerguide/token-endpoint.html However, since we are not serving static data, we need to create a fake target, and then add this to our AWS API Router, so that we can get access to the services objects.
Before, only user creds were supported by the oauth2/token route.
acwrenn
force-pushed
the
add_oauth2_token_flow
branch
from
493572b to
f3adf76
Compare
|
Hi, this would be really useful to have locally :) was there a reason the PR was not merged ? |
|
We have been using a local fork, but I would like to move off that eventually. But had forgotten about this! |
|
When will be merged? I need it! |
Cognito supports more features than just its AWS-API calls demonstrate. This service fake simulates just those AWS APIs - except for .well-known/jwks.json.
This PR starts to add support for the token endpoint outlined here: https://docs.aws.amazon.com/cognito/latest/developerguide/token-endpoint.html
However, since we are not serving static data, we need to create a fake target, and then add this to our AWS API Router, so that we can get access to the services objects.
Please give some feedback on the "not real AWS target method" - it seemed the best way to get the shared service objects to the inner handler.
BUILD NOTES: