-
-
Notifications
You must be signed in to change notification settings - Fork 4
Inception using Api Keys by config #34
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
base: master
Are you sure you want to change the base?
Conversation
| return func(next box.H) box.H { | ||
| return func(ctx context.Context) { | ||
|
|
||
| if apiKey == "" && apiSecret == "" { |
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.
Why letting the user pass if no credentials?
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.
If InceptionDB has been started without api-key then we let the user continue. Some other system will take care of this, or it is a none credentials needed scenario (testing).
It is not mandatory, as it is not in the current version.
api/0_interceptors.go
Outdated
| if key != apiKey || secret != apiSecret { | ||
| w := box.GetResponse(ctx) | ||
| w.WriteHeader(http.StatusUnauthorized) | ||
| PrettyError{ |
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.
No need to marshal and return response at this point. The error should be set (box SetError) so that anyone can marshal in other formats (html, json, xml...)
| }) | ||
| } | ||
|
|
||
| func (p PrettyError) MarshalTo(w io.Writer) error { |
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.
not needed
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.
agree
| e.Encode(c) | ||
| } | ||
|
|
||
| if c.ApiKey == "" || c.ApiSecret == "" { |
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.
I wouldnt put a warning, just a log saying Auth: disabled or enabled
| WithActions( | ||
| box.Get(statics.ServeStatics(staticsDir)).WithName("serveStatics"), | ||
| ) | ||
| if !hideUI { |
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.
Why hiding UI? Maybe we should allow to introduce input api-key/api-secret from the UI
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.
It is done as well.
alfonsocantos
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.
I have done some of the comments.
| return func(next box.H) box.H { | ||
| return func(ctx context.Context) { | ||
|
|
||
| if apiKey == "" && apiSecret == "" { |
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.
If InceptionDB has been started without api-key then we let the user continue. Some other system will take care of this, or it is a none credentials needed scenario (testing).
It is not mandatory, as it is not in the current version.
| }) | ||
| } | ||
|
|
||
| func (p PrettyError) MarshalTo(w io.Writer) error { |
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.
agree
| WithActions( | ||
| box.Get(statics.ServeStatics(staticsDir)).WithName("serveStatics"), | ||
| ) | ||
| if !hideUI { |
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.
It is done as well.
|
Fixing Unauthorized Error in progress... |
|
done ... I guess |
No description provided.