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

uninitialized constant Doorkeeper::JWT::JWK in 1.8.4 #187

Closed
brent-cybrid opened this issue Feb 1, 2023 · 8 comments · Fixed by #188
Closed

uninitialized constant Doorkeeper::JWT::JWK in 1.8.4 #187

brent-cybrid opened this issue Feb 1, 2023 · 8 comments · Fixed by #188

Comments

@brent-cybrid
Copy link

brent-cybrid commented Feb 1, 2023

I just upgraded to doorkeeper-openid_connect version 1.8.4 and my application is no longer running with the following error and stack trace:

JWT::JWK.new(key)
       ^^^^^> with backtrace:

# /Users/user/.gem/ruby/3.1.3/gems/doorkeeper-openid_connect-1.8.4/lib/doorkeeper/openid_connect.rb:51:in `signing_key'
# /Users/user/.gem/ruby/3.1.3/gems/doorkeeper-openid_connect-1.8.4/lib/doorkeeper/openid_connect/id_token.rb:35:in `as_jws_token'
@nbulaj
Copy link
Member

nbulaj commented Feb 1, 2023

And with 1.8.3 it works? Could be related to #177 ?

@brent-cybrid
Copy link
Author

brent-cybrid commented Feb 1, 2023

And with 1.8.3 it works? Could be related to #177 ?

Yes, it works with 1.8.3. Downgrading back to that version fixes the issue.

Yes, it looks related to #177. The comment here appears to be accurate. Our application does rely on json-jwt so would need to migrate to using ruby-jwt like you have in the project to try and resolve the issue.

@nbulaj
Copy link
Member

nbulaj commented Feb 1, 2023

Yeah so to fix it there are two ways:

  • Migrate to the new gem
  • Fix doorkeeper-openid_connect to support both gems (maybe @kristof-mattei will want to check)

@zavan
Copy link
Contributor

zavan commented Feb 1, 2023

I have the exact same issue but my project does not rely on json-jwt at all, it's not even in my Gemfile.lock.

Started GET "/oauth/discovery/keys" for 172.18.0.1 at 2023-02-01 19:33:53 +0000
Processing by Doorkeeper::OpenidConnect::DiscoveryController#keys as */*
Completed 500 Internal Server Error in 1ms (ActiveRecord: 0.0ms | Allocations: 152)

NameError (uninitialized constant Doorkeeper::JWT::JWK):

doorkeeper-openid_connect (1.8.4) lib/doorkeeper/openid_connect.rb:51:in `signing_key'
doorkeeper-openid_connect (1.8.4) lib/doorkeeper/openid_connect.rb:55:in `signing_key_normalized'
doorkeeper-openid_connect (1.8.4) app/controllers/doorkeeper/openid_connect/discovery_controller.rb:105:in `keys_response'

In my Gemfile I have:

gem 'doorkeeper'
gem 'doorkeeper-jwt'
gem 'doorkeeper-openid_connect'

All in the latest version.

Gemfile.lock:

    doorkeeper (5.6.4)
      railties (>= 5)
    doorkeeper-jwt (0.4.1)
      jwt (>= 2.1)
    doorkeeper-openid_connect (1.8.4)
      doorkeeper (>= 5.5, < 5.7)
      jwt (>= 2.5)
    oauth2 (2.0.9)
      faraday (>= 0.17.3, < 3.0)
      jwt (>= 1.0, < 3.0)
    ...
    jwt (2.6.0)

When I try to access the JWT::JWK class in the Rails console it works:

irb(main):004:0> JWT::JWK
=> JWT::JWK
irb(main):005:0> JWT::JWK.method(:new)
=> #<Method: JWT::JWK.new(create_from)(key, params=..., options=...) /usr/local/bundle/gems/jwt-2.6.0/lib/jwt/jwk.rb:9>

I'm even using the JWT gem in my own code and it works there:

config.jwt_jwk = JWT::JWK.new(
  pk,
  nil,
  kid_generator: ::JWT::JWK::Thumbprint
)

I also tried adding jwt to my Gemfile but same problem.

So the problem only happens inside this gem. Looks like a loading order problem, or something like that.

@zavan
Copy link
Contributor

zavan commented Feb 1, 2023

Found the problem, PR to follow.

@kristof-mattei
Copy link
Contributor

@zavan your error is different. When you have the error it is searching for Doorkeeper::JWT::JWK, which is in correct. I think we can fix your error by prefixing

with :: to ensure it doesn't search for JWT in the same namespace (i.e. Doorkeeper). This also explains why it works in the console.

Can you try changing that file and confirm if that does the trick?

@brent-cybrid can you post the complete stacktrace? Want to make sure that it is the gem's absence and not something I missed.

@nbulaj I think supporting both is another can of worms, which I find quite risky when it comes to security-related code like this. It's not even like we can specify the model the consumer would like to use, like https://doorkeeper.gitbook.io/guides/configuration/models, as the APIs are actually different. Thoughts?

@zavan
Copy link
Contributor

zavan commented Feb 1, 2023

@kristof-mattei Yeah, I tested #188 and it worked.

@nbulaj
Copy link
Member

nbulaj commented Feb 2, 2023

Released with 1.8.5

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 a pull request may close this issue.

4 participants