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

Allow extracting tokens in different ways #3

Merged
merged 5 commits into from
Mar 2, 2015
Merged

Allow extracting tokens in different ways #3

merged 5 commits into from
Mar 2, 2015

Conversation

xogeny
Copy link

@xogeny xogeny commented Feb 24, 2015

This pull request is in response to the discussion in #2. This allows the user to specify how the token is extracted from the request. This change is backward compatible with previous versions because the default choice for how tokens are extracted is the same as it was before this became configurable (i.e., to extract it from the Authorization header using the Bearer scheme.

This is a response to issue #2.  The idea here (as suggested by @jfromaniello) is to allow
the user to provide a function that takes a request and returns the extracted token (or
an error).  I've implemented three different token extraction functions as part of this
commit.  The first is FromAuthHeader which does what the previous version did (extract
the token from an Authorization header).  This is the default extraction function which
retains backward compatibility.  In addition, I also created the function FromParameter
which extracts the token from a given query string parameter.  Finally, I added a special
function called FromFirst that takes multiple token extraction function and returns the
first token or error it comes across.  This allows both extractio methods to be used
on the same request with one taking precedence over the other.
@xogeny xogeny mentioned this pull request Feb 24, 2015
errorMsg := "Authorization header format must be Bearer {token}"
m.Options.ErrorHandler(w, r, errorMsg)
return errors.New(errorMsg)
return "", fmt.Errorf("Authorization header format must be Bearer {token}")
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't this just logging to the console? I think it should return an error not just print it out.

Copy link
Author

Choose a reason for hiding this comment

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

It is returning an error. fmt.Errorf produces an error object. It does not output to console. You are thinking of fmt.Printf. fmt.Errorf is just a convenience function that does a fmt.Sprintf and then wraps the resulting string in an error object (e.g., errors.New(...)).

Copy link
Contributor

Choose a reason for hiding this comment

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

Awesome :). Thanks!

@mgonto
Copy link
Contributor

mgonto commented Feb 24, 2015

Hey,

Thanks for the PR! I've put a few comments. Let me know what you think.

Thanks!

@xogeny
Copy link
Author

xogeny commented Mar 2, 2015

I added the simplified logging statements.

@mgonto
Copy link
Contributor

mgonto commented Mar 2, 2015

Aweosme thanks!

You just missed one place where it should call the new logf function and then ready to merge.

Thanks!

@xogeny
Copy link
Author

xogeny commented Mar 2, 2015

I didn't miss it, it just wasn't in a form where logf could be used directly. But I refactored it and pushed an updated version.

@mgonto
Copy link
Contributor

mgonto commented Mar 2, 2015

Looks awesome now! Thanks for the help @xogeny.

Merging :).

mgonto added a commit that referenced this pull request Mar 2, 2015
Allow extracting tokens in different ways
@mgonto mgonto merged commit 9b05532 into auth0:master Mar 2, 2015
@drawks drawks mentioned this pull request May 3, 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.

None yet

3 participants