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

Add custom authorization header name #96

Merged
merged 1 commit into from
Nov 24, 2015

Conversation

pdoreau
Copy link
Contributor

@pdoreau pdoreau commented Oct 6, 2015

Hi, I need double authentication : HTTP basic for general restriction and JWT at API level.
They both used the same header (Authorization).
This PR allows to use a custom header name for JWT token in order to handle double authentication.

*/
public function __construct($prefix)
public function __construct($prefix, $name)
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be safer to set a default value of 'Autorization' for the $name argument, to prevent any BC break.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Default $name is set in JWTFactory.php, the same way default $prefix is defined

@slashfan
Copy link
Contributor

slashfan commented Oct 9, 2015

Hi, nice addition for special cases.

*/
public function __construct($prefix)
public function __construct($prefix, $name)
Copy link
Contributor

Choose a reason for hiding this comment

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

Again, I think there should be a default value for the $name parameter, even if it is overwritable by the dependency injector like any other argument. The class is called AuthorizationHeaderTokenExtractor, so it would make sense to have it "preconfigured" for this header name.

Maybe a constant DEFAULT_HEADER_NAME = 'Authorization' and a constructor like __construct($prefix, $name = self::DEFAULT_HEADER_NAME).

In a future major version, the class could be renamed to HeaderTokenExtractor.

slashfan added a commit that referenced this pull request Nov 24, 2015
Add custom authorization header name
@slashfan slashfan merged commit 12ccf15 into lexik:master Nov 24, 2015
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