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

Improvement on run.NewServer related to meta.QueryAuthorizer. #5787

Merged
merged 2 commits into from
Feb 23, 2016

Conversation

chris-ramon
Copy link
Contributor

Details:
if h.requireAuthentication {
    if err := h.QueryAuthorizer.AuthorizeQuery(user, query, db); err != nil {
        if err, ok := err.(meta.ErrAuthorize); ok {
            h.Logger.Printf("unauthorized request | user: %q | query: %q | database %q\n", err.User, err.Query.String(), err.Database)
        }
        httpError(w, "error authorizing query: "+err.Error(), pretty, http.StatusUnauthorized)
        return
    }
}
  • When run.NewServer wires all the services; and it creates new instance of httpd.Service, it does not set *Handler.QueryAuthorizer (due some reason I might be missing? cc/ @benbjohnson) - due this, performing http requests against run.Server will throw a run time error: invalid memory address or nil pointer dereference, because *Handler.QueryAuthorizer is nil.
  • This PR updates run.Server.appendHTTPDService to set that missing QueryAuthorizer explained above.
  • CHANGELOG.md updated
  • Rebased/mergable
  • Tests pass
  • Sign CLA

@chris-ramon chris-ramon changed the title adds missing srv.Handler.QueryAuthorizer Improvement on run.NewServer related to meta.QueryAuthorizer. Feb 23, 2016
@jwilder
Copy link
Contributor

jwilder commented Feb 23, 2016

👍 Thanks @chris-ramon!

jwilder added a commit that referenced this pull request Feb 23, 2016
Improvement on `run.NewServer` related to `meta.QueryAuthorizer`.
@jwilder jwilder merged commit 92ae2a0 into influxdata:master Feb 23, 2016
@jwilder jwilder added this to the 0.11.0 milestone Feb 23, 2016
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 this pull request may close these issues.

2 participants