ZCS-19497: Add IRopcCredCache with Guava cache and SSHA512 hashing for non-EAS protocol credential caching - #1850
Conversation
44cd9a0 to
3cca7cf
Compare
There was a problem hiding this comment.
Remaining comments which cannot be posted as a review comment to avoid GitHub Rate Limit
checkstyle
🚫 [checkstyle] <com.puppycrawl.tools.checkstyle.checks.javadoc.JavadocMethodCheck> reported by reviewdog 🐶
Expected @PARAM tag for 'ip'.
🚫 [checkstyle] <com.puppycrawl.tools.checkstyle.checks.javadoc.JavadocMethodCheck> reported by reviewdog 🐶
Expected @PARAM tag for 'deviceId'.
🚫 [checkstyle] <com.puppycrawl.tools.checkstyle.checks.blocks.NeedBracesCheck> reported by reviewdog 🐶
'if' construct must use '{}'s.
🚫 [checkstyle] <com.puppycrawl.tools.checkstyle.checks.javadoc.JavadocMethodCheck> reported by reviewdog 🐶
Expected @PARAM tag for 'email'.
🚫 [checkstyle] <com.puppycrawl.tools.checkstyle.checks.javadoc.JavadocMethodCheck> reported by reviewdog 🐶
Expected @PARAM tag for 'userAgent'.
🚫 [checkstyle] <com.puppycrawl.tools.checkstyle.checks.javadoc.JavadocMethodCheck> reported by reviewdog 🐶
Expected @PARAM tag for 'protocol'.
🚫 [checkstyle] <com.puppycrawl.tools.checkstyle.checks.javadoc.JavadocMethodCheck> reported by reviewdog 🐶
Expected @PARAM tag for 'provider'.
🚫 [checkstyle] <com.puppycrawl.tools.checkstyle.checks.javadoc.JavadocMethodCheck> reported by reviewdog 🐶
Expected @PARAM tag for 'ip'.
🚫 [checkstyle] <com.puppycrawl.tools.checkstyle.checks.javadoc.JavadocMethodCheck> reported by reviewdog 🐶
Expected @PARAM tag for 'deviceId'.
🚫 [checkstyle] <com.puppycrawl.tools.checkstyle.checks.blocks.NeedBracesCheck> reported by reviewdog 🐶
'if' construct must use '{}'s.
🚫 [checkstyle] <com.puppycrawl.tools.checkstyle.checks.javadoc.JavadocMethodCheck> reported by reviewdog 🐶
Expected @PARAM tag for 'email'.
🚫 [checkstyle] <com.puppycrawl.tools.checkstyle.checks.javadoc.JavadocMethodCheck> reported by reviewdog 🐶
Expected @PARAM tag for 'userAgent'.
🚫 [checkstyle] <com.puppycrawl.tools.checkstyle.checks.javadoc.JavadocMethodCheck> reported by reviewdog 🐶
Expected @PARAM tag for 'protocol'.
🚫 [checkstyle] <com.puppycrawl.tools.checkstyle.checks.javadoc.JavadocMethodCheck> reported by reviewdog 🐶
Expected @PARAM tag for 'provider'.
🚫 [checkstyle] <com.puppycrawl.tools.checkstyle.checks.javadoc.JavadocMethodCheck> reported by reviewdog 🐶
Expected @PARAM tag for 'ip'.
🚫 [checkstyle] <com.puppycrawl.tools.checkstyle.checks.javadoc.JavadocMethodCheck> reported by reviewdog 🐶
Expected @PARAM tag for 'deviceId'.
🚫 [checkstyle] <com.puppycrawl.tools.checkstyle.checks.blocks.NeedBracesCheck> reported by reviewdog 🐶
'if' construct must use '{}'s.
🚫 [checkstyle] <com.puppycrawl.tools.checkstyle.checks.javadoc.JavadocMethodCheck> reported by reviewdog 🐶
Expected @PARAM tag for 'email'.
🚫 [checkstyle] <com.puppycrawl.tools.checkstyle.checks.javadoc.JavadocMethodCheck> reported by reviewdog 🐶
Expected @PARAM tag for 'userAgent'.
🚫 [checkstyle] <com.puppycrawl.tools.checkstyle.checks.javadoc.JavadocMethodCheck> reported by reviewdog 🐶
Expected @PARAM tag for 'protocol'.
🚫 [checkstyle] <com.puppycrawl.tools.checkstyle.checks.javadoc.JavadocMethodCheck> reported by reviewdog 🐶
Expected @PARAM tag for 'provider'.
🚫 [checkstyle] <com.puppycrawl.tools.checkstyle.checks.javadoc.JavadocMethodCheck> reported by reviewdog 🐶
Expected @PARAM tag for 'ip'.
🚫 [checkstyle] <com.puppycrawl.tools.checkstyle.checks.javadoc.JavadocMethodCheck> reported by reviewdog 🐶
Expected @PARAM tag for 'deviceId'.
🚫 [checkstyle] <com.puppycrawl.tools.checkstyle.checks.blocks.NeedBracesCheck> reported by reviewdog 🐶
'if' construct must use '{}'s.
🚫 [checkstyle] <com.puppycrawl.tools.checkstyle.checks.regexp.RegexpSinglelineJavaCheck> reported by reviewdog 🐶
Line exceeds 120 characters in comments
🚫 [checkstyle] <com.puppycrawl.tools.checkstyle.checks.sizes.LineLengthCheck> reported by reviewdog 🐶
Line is longer than 120 characters (found 127).
🚫 [checkstyle] <com.puppycrawl.tools.checkstyle.checks.blocks.NeedBracesCheck> reported by reviewdog 🐶
'if' construct must use '{}'s.
There was a problem hiding this comment.
Remaining comments which cannot be posted as a review comment to avoid GitHub Rate Limit
checkstyle
🚫 [checkstyle] <com.puppycrawl.tools.checkstyle.checks.blocks.NeedBracesCheck> reported by reviewdog 🐶
'if' construct must use '{}'s.
🚫 [checkstyle] <com.puppycrawl.tools.checkstyle.checks.regexp.RegexpSinglelineJavaCheck> reported by reviewdog 🐶
Line exceeds 120 characters in comments
🚫 [checkstyle] <com.puppycrawl.tools.checkstyle.checks.sizes.LineLengthCheck> reported by reviewdog 🐶
Line is longer than 120 characters (found 127).
🚫 [checkstyle] <com.puppycrawl.tools.checkstyle.checks.blocks.NeedBracesCheck> reported by reviewdog 🐶
'if' construct must use '{}'s.
| import com.zimbra.cs.account.auth.PasswordUtil.SSHA512; | ||
| import java.util.concurrent.TimeUnit; | ||
|
|
||
| import static com.zimbra.cs.account.auth.ropc.IRopcConstants.TTL_MS; |
There was a problem hiding this comment.
🚫 [checkstyle] <com.puppycrawl.tools.checkstyle.checks.imports.ImportOrderCheck> reported by reviewdog 🐶
Extra separation in import group before 'com.zimbra.cs.account.auth.ropc.IRopcConstants.TTL_MS'
| String effectivePassword = getEffectivePassword(password); | ||
| String key = buildKey(email, userAgent, protocol, provider, ip, deviceId); | ||
|
|
||
| // Store the successful credential |
There was a problem hiding this comment.
comments to start with lower case here and below
| user, protocolStr, userAgent, deviceId, ipAddress, provider); | ||
|
|
||
| // Implementation of the cache check using composite keys | ||
| if (useCustomCache) { |
There was a problem hiding this comment.
This check is not required , as this flow works only if custom auth is idp ropc.
| CRED_CACHE.put(key, entry); | ||
|
|
||
| // Store IP bridge when deviceId is present | ||
| // Enables OPCC to real deviceId upgrade in isValid() |
| default: | ||
| throw ServiceException.FAILURE("Authentication service temporarily unavailable.", null); | ||
| } | ||
| long expires_in = 3600000L; |
There was a problem hiding this comment.
🚫 [checkstyle] <com.puppycrawl.tools.checkstyle.checks.naming.LocalVariableNameCheck> reported by reviewdog 🐶
Name 'expires_in' must match pattern '^[a-z][a-zA-Z0-9]*$'.
| */ | ||
| private static final class CacheEntry { | ||
|
|
||
| final String hash; |
There was a problem hiding this comment.
🚫 [checkstyle] <com.puppycrawl.tools.checkstyle.checks.design.VisibilityModifierCheck> reported by reviewdog 🐶
Variable 'hash' must be private and have accessor methods.
| * absolute wall-clock timestamp (ms) after which this entry is considered expired. | ||
| * computed as: System.currentTimeMillis() + (expiresInSeconds * 1000) | ||
| */ | ||
| final long expiryTimestamp; |
There was a problem hiding this comment.
🚫 [checkstyle] <com.puppycrawl.tools.checkstyle.checks.design.VisibilityModifierCheck> reported by reviewdog 🐶
Variable 'expiryTimestamp' must be private and have accessor methods.
| /** | ||
| * validates entry against password. Immediately invalidates expired entries. | ||
| */ | ||
| private static boolean isMatch(CacheEntry entry, String key, String password) { |
| /** | ||
| * validates entry against password. Immediately invalidates expired entries. | ||
| */ | ||
| private static boolean isMatch(CacheEntry entry, String key, String password) { |
| /** | ||
| * validates entry against password. Immediately invalidates expired entries. | ||
| */ | ||
| private static boolean isMatch(CacheEntry entry, String key, String password) { |
| /** | ||
| * validates entry against password. Immediately invalidates expired entries. | ||
| */ | ||
| private static boolean isMatch(CacheEntry entry, String key, String password) { |
…r EAS protocol credential caching
08c057f to
5fdd816
Compare
8fb6ac3 to
e9dedfa
Compare
ZCS-19497: Add Guava credential cache for IRopc non-EAS protocols