-
Notifications
You must be signed in to change notification settings - Fork 0
auth: improve TokenHandling #19
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
Conversation
|
hi @en-milie @Stranger6667 , I added some fields to the |
Stranger6667
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me!
|
To be honest I'll make this a bit more generic and easier to extend for the future. Something like: TokenHandling:
type: object
properties:
extractFrom:
type: string
enum: ["body", "header"]
extractPath:
type: string
description: "JSON Pointer for body extraction, or header name for header extraction"
sendIn:
type: string
enum: ["header", "query", "cookie"]
sendName:
type: string
description: "Header/query/cookie name where token should be sent"
sendFormat:
type: string
description: "Template with {token} placeholder. Default: '{token}'"
default: "{token}"
required: ["extractFrom", "extractPath", "sendIn", "sendName"] |
|
hi @ludovicianul , thanks for your input. Let me discuss it point by point:
that's good
having same field for body and header handling is fine, but I do not particularly like the name
good for
the idea was that, if you expect a cookie, you can just use a boolean to say send back all cookies received when doing authenticated calls. or are the cases in which this would be too limited?
yep, that's a good idea, and likely easier to handle than separated suffix and prefix. @Stranger6667 what do you think? |
|
Yes, apologies, I've missed the |
|
@ludovicianul @Stranger6667 |
|
Looks good! |
Stranger6667
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor bikeshedding with the property name, but otherwise it is a really nice improvement :)
| type: string | ||
| examples: | ||
| - "Authorization" | ||
| sendFormat: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: sendTemplate maybe? as it is described as a template below.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Stranger6667 good point. i updated it
|
@Stranger6667 @ludovicianul recall that, while going through the specs, feel free to suggests improvements/changes. we can then discuss them |
No description provided.