-
Notifications
You must be signed in to change notification settings - Fork 44
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 configuration option "use_application_secret" #3
Add configuration option "use_application_secret" #3
Conversation
encryption_method :rs512 | ||
end | ||
|
||
token = Doorkeeper::JWT.generate({application: {secret: secret_key}}) |
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.
Redundant curly braces around a hash parameter.
Space inside { missing.
Space inside } missing.
+1 |
@@ -18,7 +18,11 @@ def self.token_payload(opts = {}) | |||
Doorkeeper::JWT.configuration.token_payload.call opts | |||
end | |||
|
|||
def self.secret_key | |||
def self.secret_key(opts) |
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.
private
(on line 15) does not make singleton methods private. Use private_class_method
or private
inside a class << self
block instead.
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.
@chriswarren I'm not sure if you want me to change this one or not... I noticed that hound was complaining about single-quotes which are being used often in other files.
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.
Don't worry about it.
expect(subject.use_application_secret).to be false | ||
end | ||
|
||
it 'changes the value of secret_key to the applicatin\'s secret' do |
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.
application - missing the O
Looks good. Can you fix the typo in the test description and then I'll get it merged in? Thanks for adding this. |
Thanks @chriswarren |
@chriswarren would love to see this merged. I have immediate use for this fix. Thanks! |
This should allow the configuration setting
use_application_secret
which will then use the grant token's application secret when generating the JWT. I believe this was mentioned in Issue #2