Maybe check signatures first?

[utkarsh2102]

Hi @tobiashm,
Just to be extra secure, doesn't it make sense to check signatures first so that no third party could interfere?
Something like:

--- a/lib/openid/consumer/idres.rb
+++ b/lib/openid/consumer/idres.rb
@@ -70,9 +70,9 @@
       # 'endpoint', 'message', and 'signed_fields' contain the
       # verified information.
       def id_res
+        check_signature
         check_for_fields
         verify_return_to
-        check_signature
         check_nonce
         verify_discovery_results
       end

My take is that this method will raise ProtocolError in the very first place (which is a good thing!) unless the request is a valid id_res response. Once it has been verified, the methods endpoint, message, and signed_fields contain the verified information.
Thereby making everything secure from any third party interference.

I'll be happy to raise a PR if this makes sense to you?

[tobiashm]

That sounds reasonable.
It's been a while since I've looked at the spec, so I can't remember if there's anything in there about the order.

A PR would be much appreciated 😃 — and it would be great if you could verify that we're still following the spec.

Perhaps the check_nonce should also be moved if that makes sense (just from the names, it looks like it could make sense to first perform all the checks, then verify URLs).