search

fabric8-services / fabric8-auth

Identity and Access Management for fabric8 services
https://auth.openshift.io/api/status
Apache License 2.0
14 stars 26 forks source link

exclude already-banned users from deactivation #809

Closed xcoulon closed 5 years ago

xcoulon commented 5 years ago

no need to notify and later deactivate users who were already banned. Even more, their accounts MUST remain intact, so we can keep a trace of their ban and prevent them from joining again.

Fixes #ODC359

Signed-off-by: Xavier Coulon xcoulon@redhat.com

xcoulon commented 5 years ago

@sbryzak @alexeykazakov I added an SQL migration script to add an index on the banned column of the users table but I'm not sure it will be used in the context of such a query:

SELECT "identities".* FROM "identities" 
left join users on identities.user_id = users.id WHERE "identities"."deleted_at" IS NULL AND
((last_active < '2019-02-23 16:03:52' AND deactivation_notification IS NULL) 
  AND (users.banned is false)) 
ORDER BY last_active, created_at

could you confirm it's worth having such a new index, please?

codecov[bot] commented 5 years ago

Codecov Report

Merging #809 into master will increase coverage by 0.01%. The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #809      +/-   ##
==========================================
+ Coverage   78.47%   78.49%   +0.01%     
==========================================
  Files          94       94              
  Lines        8943     8951       +8     
==========================================
+ Hits         7018     7026       +8     
  Misses       1405     1405              
  Partials      520      520
Impacted Files Coverage Δ
authentication/account/repository/identity.go 77.16% <100%> (+0.29%) :arrow_up:
migration/migration.go 66.66% <100%> (+0.19%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update ac601e0...03cf8b8. Read the comment docs.

xcoulon commented 5 years ago

great, thanks for your reviews @sbryzak and @alexeykazakov 🙌

xcoulon commented 5 years ago

actually, this PR fixes ODC352, not ODC359 as mentioned in the commit msg 😬