Hash shared secret before checking validity of IRS Attempts API token#7888
Hash shared secret before checking validity of IRS Attempts API token#7888mitchellhenke merged 4 commits intomainfrom
Conversation
lib/identity_config.rb
Outdated
There was a problem hiding this comment.
this line is giving me bad vibes, I am really worried about the portability of secrets referencing each other during initialization like this. How would you feel about just hardcoding our production config cost here, since it's not likely to change anytime soon?
There was a problem hiding this comment.
The pain point is testing and local development where it'd hash the production value on startup every time. Maybe there's a different place to hold it.
There was a problem hiding this comment.
Agreed on the bad vibes though
There was a problem hiding this comment.
re: testing where it hashes the prod value on startup each time, I think that was why in my commit I redid re-stubbed the config value with a known new plaintext in the controller spec
lib/identity_config.rb
Outdated
There was a problem hiding this comment.
this keep the original unhashed value in memory, right? I see that we're using it in specs, but I feel like the point of this PR was to remove that?
There was a problem hiding this comment.
they all have the same cost/salt right? so we could scrypt the incoming value once?
Maybe inside the processing block, we reorganize the secret value so it's more like a dictionary? or even a struct?
{
cost: cost,
salt: salt,
hashed_tokens: ['a', 'b', 'c']
}
There was a problem hiding this comment.
yep, that makes sense.
mitchellhenke
force-pushed
the
mitchellhenke/hash-shared-secret-check
branch
2 times, most recently
from
ce3998c to
de7a4cf
Compare
lib/identity_config.rb
Outdated
There was a problem hiding this comment.
I think we can back out this change now that we're no longer using it
changelog: Internal, IRS Attempts API, Scrypt hash token before doing a strict string comparison against valid tokens
mitchellhenke
force-pushed
the
mitchellhenke/hash-shared-secret-check
branch
from
de7a4cf to
404c5e3
Compare
2 participants
🛠 Summary of changes
This is generally not how one uses scrypt hashing, but we do not store the hashed version currently (though we may in the future and this puts the pieces in place).