Skip to content
This repository has been archived by the owner on Jun 14, 2021. It is now read-only.

feat: add authentication support #45

Closed
wants to merge 8 commits into from
Closed

Conversation

rodmatos
Copy link
Collaborator

@rodmatos rodmatos commented Jun 6, 2019

This pull request includes the integration between OpenAPI's security functionality and a client provided middleware that is executed on the routes that were marked with security.

Please refer to the docs/README.md for any usage questions.

if (req.headers.authorization) {
next()
} else {
res.status(401).json({ error: { message: "unauthorized" }})
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally the library user doesn't want to do this, but instead generate an error e.g.

new ServerUnauthorizedError({ typedPayload: '' })

that contains the possible error codes and response bodies defined in the swagger schema.
Can we showcase this somehow? Maybe we need to extend some of the type generation things for that?
I think the only response that is not in the swagger schema and still allowed is 500 with an empty response body.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that's the part we actually need to implement for this, so let's do it the next time! :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah. We have to generate those types if declared in OpenAPI file. Will work on that!

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI these types are generated now :)

server/src/Slushy.ts Outdated Show resolved Hide resolved
aimed
aimed previously approved these changes Jun 6, 2019
Copy link
Owner

@aimed aimed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🥇 niceee!

@rodmatos
Copy link
Collaborator Author

rodmatos commented Jun 9, 2021

@aimed this is appearing in my pull requests for a while. decided that it was the time to ask: shall we do this or can I close it? are you actively supporting the project?

@aimed
Copy link
Owner

aimed commented Jun 14, 2021

@rodmatos AHAHAH I just found it there too, this can be closed 🤗

@aimed aimed closed this Jun 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants