buzzfeed / sso

sso, aka S.S.Octopus, aka octoboi, is a single sign-on solution for securing internal services
MIT License
3.1k stars 186 forks source link

fillcache pkg: trigger cache update immediately #271

Closed Jusshersmith closed 4 years ago

Jusshersmith commented 5 years ago

Problem

The fillcache package is what we use with the Google (and soon to be Cognito) providers to keep group membership cached.

Once a request goes through to sso_auth to validate a users group membership, the ValidateGroup function looks in the local cache for the relevant groups: https://github.com/buzzfeed/sso/blob/14f2a962d995bbd4b69da4420df5fe53dc3468c2/internal/auth/providers/google.go#L366-L370 If the group doesn't exist in the cache, it triggers a refresh of the cache which in turn kicks off a goroutine to keep the group cache up to date (based off an input TTL).

Right now, the triggered goroutine only fills the cache after the given TTL, not immediately. Any subsequent calls to ValidateGroupMembership between the first call, and the first tick of ttl won’t have the groups cached.

Solution

Break out the logic that the goroutine uses to fill the cache into a separate method, and call it once before the loop, and then upon each tick of ttl.

Going to look at updating this PR with some logging examples to better verify the bug + expected behaviour

Notes

codecov[bot] commented 5 years ago

Codecov Report

Merging #271 into master will increase coverage by 0.32%. The diff coverage is 68.42%.

@@            Coverage Diff             @@
##           master     #271      +/-   ##
==========================================
+ Coverage   62.21%   62.54%   +0.32%     
==========================================
  Files          54       54              
  Lines        4200     4202       +2     
==========================================
+ Hits         2613     2628      +15     
+ Misses       1399     1385      -14     
- Partials      188      189       +1
Impacted Files Coverage Δ
internal/pkg/groups/mock_cache.go 0% <0%> (ø) :arrow_up:
internal/pkg/groups/fillcache.go 73.91% <76.47%> (+20.18%) :arrow_up:
Jusshersmith commented 4 years ago

Now that https://github.com/buzzfeed/sso/pull/273 is merged, we're back and once again ✅!