Change error on unimplemented party setter

[Vagab]

I think NotImplementedError with class name reads better. Also, the message was saying relying_party which is confusing in this context, since even if it's implemented we'll still get an error.

On a separate note, @tcannonfodder, why don't we just write a default implementation in, say, Warden::WebAuthn::StrategyHelpers? I don't think many users will have a better use for it than

def set_relying_party_in_request_env
  request.env[relying_party_key] = relying_party
end

Happy to make a pr

[tcannonfodder]

I also need to review the updated error message when I'm at my desk

[tcannonfodder]

As annoying & confusing as it can be, there are both semantic & practical reasons to not use NotImplentedError, see: https://oleg0potapov.medium.com/ruby-notimplementederror-dont-use-it-dff1fd7228e5.

Some possible candidates

• Leave as-is
• RuntimeError
• NoMethodError

[Vagab]

Oooh, ok. Good catch. Fixed

[tcannonfodder]

On a separate note, @tcannonfodder, why don't we just write a default implementation in, say, Warden::WebAuthn::StrategyHelpers? I don't think many users will have a better use for it than

def set_relying_party_in_request_env

  request.env[relying_party_key] = relying_party

end

Ack, I know I had this conversation with @heliocola, but I can't find it 😬

The gist is that I don't want this gem to be too magical, which is one of the problems I have with devise as a whole.

Since the relying party is the core for the entire WebAuthn flow; the app should be responsible for:

• Setting it up as needed for each request (even if it remains constant)
• Slotting it into the request environment so that the warden strategy and this gem can use it

I know it can feel like a bit of overkill, but it has the benefits of:

• Being easy to search for in your codebase
• Being able to easily debug without having to pry open & dig into a gem
• Reducing the maintenance footprint of this gem, since it's less code we manage (and the error message is clear as to what's needed)

[Vagab]

The gist is that I don't want this gem to be too magical, which is one of the problems I have with devise as a whole.

totally agree with this statement. However, I think this is the perfect candidate(there are few) to actually have a generic implementation for. Mainly because I don't see how the implementation for it could reasonably be any different.

[tcannonfodder]

Mainly because I don't see how the implementation for it could reasonably be any different.

Hmmm; let me think on that, you make a good point.

I'm also curious about those other places you think a generic implementation would be useful; feel free to make an issue!

[tcannonfodder]

@Vagab I actually moved set_relying_party_in_request_env into Warden::WebAuthn::RackHelper in #29 (7b7d501).

Can you close this PR and make a new one based on that branch, with this error message being on the relying_party method directly, see:
7b7d501#diff-c88c685935abcec7655c75a59b34681909226e6fa4088f253427d5ddefef1c99R72