Simo Nikula (Migrated from SEC-1753) said:
discovered may be null which causes NullPointerException
// retrieve the previously stored discovery information
DiscoveryInformation discovered = (DiscoveryInformation) request.getSession().getAttribute(DISCOVERY_INFO_KEY);
List<OpenIDAttribute> attributesToFetch = (List<OpenIDAttribute>) request.getSession().getAttribute(ATTRIBUTE_LIST_KEY);
Identifier id = discovered.getClaimedIdentifier();
Luke Taylor said:
Could you clarify the steps which lead to this behaviour please.
Simo Nikula said:
This case happens during hacking/cracking attempt (thats why it is minor).
I was comparing openid packages and checked how they handle case where response from OpenID Provider is replayed.
Security is ok in above implementation as data is removed from session after it has been used but diagnostics from NullPointerException is not too good.
You may have better idea but something like
throw new OpenIDConsumerException("DiscoveryInformation is not available, Possible causes are e.g. lost session or replay attack");
Other option that I prefer would be not to remove DiscoveryInformation from session but let ConsumerManager._nonceVerifier.seen() report possible attack
OK, I've added a check for the missing DiscoveryInformation as you suggest. If you want to retain the DiscoveryInformation in the session for the duration you can override the endConsumption method and put it back after calling super.