cedarcode / webauthn-ruby

WebAuthn ruby server library ― Make your Ruby/Rails web server become a conformant WebAuthn Relying Party
https://rubygems.org/gems/webauthn
MIT License
649 stars 53 forks source link

Inconsistent/unexpected naming (`Webauthn` vs `WebAuthn`) #352

Open danielolivaresd opened 2 years ago

danielolivaresd commented 2 years ago

There are certain setups that will try to do loading for us (e.g. Rails, or more specifically, Zeitwerk) which infer that if we have a webauthn.rb file, it will define a Webauthn module (not WebAuthn, as it currently does).

I noted that a Sidekiq worker from my Rails app was raising a NameError (uninitialized constant Webauthn). ~I could fix this by appending require: false to my Gemfile entry for this gem and manually calling require 'webauthn' in the places where it was needed (e.g. initializer, controllers).~

Note: In my app I also have controllers such as Webauthn::CredentialsController in app/controllers/webauthn/credentials_controller.rb, and this could have been the cause of my particular error. Nevertheless, I think we could all benefit from sticking to a consistent naming from this gem.

bdewater commented 2 years ago

I believe this is caused by the filename in your app as you mentioned. Zeitwerk is used to (re)load your Rails app code, not gems. From https://guides.rubyonrails.org/v7.0/autoloading_and_reloading_constants.html:

Idiomatic Rails applications only issue require calls to load stuff from their lib directory, the Ruby standard library, Ruby gems, etc. That is, anything that does not belong to their autoload paths, explained below.

Within an autoload path, file names must match the constants they define as documented here. By default, the autoload paths of an application consist of all the subdirectories of app that exist when the application boots ---except for assets, javascript, and views--- plus the autoload paths of engines it might depend on.

danielolivaresd commented 2 years ago

Yeah, I also believe it had to do with my app's Webauthn module.

Even if I still feel that following a more standard naming convention (i.e. webauthn.rb declaring Webauthn and web_authn.rb declaring WebAuthn) could be clearer, it's not a huge thing, so feel free to close this issue if you want (I'm not closing it in case it is something you want to have in the roadmap or would accept a PR about).