fix failure counter and improve the way VP and Nginx pass error information back and forth during a login session

[inklesspen]

I've been reading login.go and I think the failure counter logic might not be working properly since #350.

The session is locked to the path /auth/{state}/, so the failure counter set in line 84 will surely not be available to the login handler, right?

[bnfinet]

Yeah I think you're right.

Failure counter is meant to be path specific. It's meant to check if the current path being accessed is in an infinite loop. It may be broken, I don't recall triggering it personally in some time.

I've been thinking for a while that the failure counter and possibly error items being passed back to NGINX should go away and be replaced by something like a struct that gets passed to from VP to nginx and then passed back in. This struct could grow (or shrink!) as VP changes over time and it wouldn't require any changes to the nginx config. This would allow for a timestamp to be carried and perhaps the struct could be signed using the same secret that VP uses for it's JWT.

[inklesspen]

Best not to reuse secrets for multiple purposes, especially if there's a chance an attacker could introduce their chosen plaintext.

[bnfinet]

@inklesspen could you please clarify, what's the problem with using a secret for the same purpose (signing) in multiple places?

[inklesspen]

I'm not really an expert myself, but my understanding is that if you use the same key for your signin state and for the jwt, and there is some way an attacker can control the signin state, this could enable the attacker to plug arbitrary data into the signed JWT.