> Also, why is this defined in ValidatorModuleResult?  If I interpret Zsolt's
> comment upthread correctly it was meant to be placed in 
> OAuthValidatorCallbacks
> as a new callback - which I agree with would be better.  The question of where
> and when to invoke it remains, but whatever we build I think it should be auth
> agnostic so that any auth can be hooked into it.

+1 Yes, that's what I meant. Also agree with the unnecessary expiry
timestamp with the callback - if we want to store some additional data
for the checks, that should be a void*, but with the single threaded
model there's no real need for it, the validator can store anything it
needs in global variables.

> 2. Terminating sessions with expired/revoked tokens before executing new
> commands.

> Token expiration is IMHO not a use case for a FATAL error, if we want to
> terminate a connection we can do it in a more graceful way.

There are different reasons for token expiration, one is a simple
timeout where all we have to do is communicate to the client that we
need a refresh (gracefully), and the other is where a token is
immediately revoked because of a security incident, in which case
immediate termination is a good practice.

There was an earlier discussion about it in the password expiration
thread[1], and the possible use of the GoAway[2] for it. Jacob
suggested making it user configurable, which seems like a reasonable
way to do it.

I also wanted to work on this patch, but I didn't start working on it
so far, because I wanted to see where those threads go, as the changes
introduced in them could be useful for the oauth token
refresh/disconnection mechanism.

[1] : 
https://www.postgresql.org/message-id/flat/CAER375OvH3_ONmc-SgUFpA6gv_d6eNj2KdZktzo-f_uqNwwWNw%40mail.gmail.com
[2] : 
https://www.postgresql.org/message-id/flat/DDPQ1RV5FE9U.I2WW34NGRD8Z%40jeltef.nl


Reply via email to