FR-12682 redis entitlements cache by arturwolny-frontegg · Pull Request #161 · frontegg/nodejs-sdk

arturwolny-frontegg · 2023-07-31T08:58:08Z

No description provided.

…e manager implementations BREAKING CHANGE: removed accessTokenOptions from FronteggContext configuration

…ken section

test(context): fixed tests of FronteggContext

refactor(cache): decoupled cache managers from AccessTokens

# [6.0.0-alpha.1](5.1.1-alpha.1...6.0.0-alpha.1) (2023-07-31) ### Code Refactoring * **sdk:** removed irrelevant accessTokenOptions; refactored cache manager implementations ([3bbe939](3bbe939)) ### Bug Fixes * **cache:** Bringing back the ICacheManager generic to the class level ([7d04440](7d04440)) ### Features * **cache:** decoupled cache managers from AccessTokens ([85db523](85db523)) ### BREAKING CHANGES * **sdk:** removed accessTokenOptions from FronteggContext configuration

…maps

…l FronteggSDK cache

… frontegg cache component

src/clients/entitlements/entitlements-app.ts

src/clients/entitlements/entitlements-client.ts

src/components/frontegg-context/types.ts

src/components/leader-election/ioredis.lock-handler.ts

…update

src/clients/entitlements/entitlements-app.ts

yossi-frontegg · 2023-08-17T09:35:03Z

src/clients/entitlements/storage/cache.revision-manager.ts

+
+  constructor(private readonly cache: ICacheManager<CacheValue>) {}
+
+  async loadSnapshotAsCurrent(dto: VendorEntitlementsDto): Promise<IsUpdatedToRev> {


"loadSnapshotAsCurrent" - what is that "Current"? the name is not fully clear to me
maybe "loadSnapshotAsCurrentRevision" would be more explanatory

I had these concerns as well, I found it very difficult to come up with self-explanatory and fancy names for methods. Seems like I failed in both aspects 😮‍💨

yossi-frontegg · 2023-08-17T10:49:51Z

src/clients/entitlements/storage/frontegg-cache/frontegg.cache-initializer.ts

+
+    const cacheInitializer = new FronteggEntitlementsCacheInitializer(entitlementsCache);
+
+    const sources = new DtoToCacheSourcesMapper().map(dto);


Why not creating the mapper with static map function?

yossi-frontegg · 2023-08-17T11:32:14Z

src/components/cache/managers/ioredis/ioredis-cache.manager.spec.ts

+import IORedis from 'ioredis';
+import { CacheValue } from '../cache.manager.interface';
+
+// TODO:  define all tests of Redis-based ICacheManager implementations in single file, only change the implementation


still need that todo?

yes, it's about having a single .spec file for all ICacheManager instances, as they're about to do the same from the interface perspective

yossi-frontegg · 2023-08-17T11:33:33Z

src/components/cache/managers/ioredis/ioredis-cache.manager.ts

+    }
+  }
+
+  forScope<S extends CacheValue>(prefix?: string): ICacheManager<S> {


Shouldn't this function be static?

Well, no. The intention was to create the new (scoped) instance of the same class, using the very same Redis connection (or NodeCache instance in case of local), but with different prefix and (important) - different typing.

When you use FronteggCache.getInstance() static method, in return you're receiving the instance and you don't really know (or - don't really NEED TO KNOW) what class it is. It's just fulfilling the ICacheManager interface. If this method is static, you'd need to iterate over known implementation and check which instance it is - and that's an antipattern, because in your code you need to know about the ICacheManager, not about particular implementations.

Usage of non-static method allows to apply the polymorphism here, so cache implementations know how to actually produce their scoped copies and don' leak implementation details that other parts of code should not be concerned about.

yossi-frontegg · 2023-08-17T11:39:23Z

src/components/cache/managers/redis/redis-cache.collection.ts

+import { ICacheValueSerializer } from '../../serializers/types';
+import { CacheValue, ICacheManagerCollection } from '../cache.manager.interface';
+
+export class RedisCacheCollection implements ICacheManagerCollection<CacheValue> {


Can you explain the difference between this and the IORedis one?

it uses different Redis library!

src/components/leader-election/factory.ts

yossi-frontegg · 2023-08-17T11:55:51Z

src/components/leader-election/index.ts

+      this.options.expireInMs,
+    );
+
+    this.scheduleLeadershipTry();


why not calling this as part of becomeFollower function?

The becomeFollower method should be called when the follower/leader status flips. When instance was a follower and still remains a follower, this method should not be called.

Why? I don't want to emit follower event after each leadership race that is lost.

yossi-frontegg · 2023-08-17T12:58:24Z

src/components/leader-election/ioredis.lock-handler.ts

+  private static EXTEND_LEADERSHIP_SCRIPT =
+    "if redis.call('GET', KEYS[1]) == ARGV[1] then return redis.call('PEXPIRE', KEYS[1], ARGV[2]) else return 0 end";
+
+  async tryToMaintainTheLock(key: string, value: string, expirationTimeMs: number): Promise<boolean> {


Can you add here a comment explaining how this works?

cycode-security · 2024-03-17T17:30:21Z

src/utils/index.ts

-    Logger.debug(`trying again in ${delayTime} seconds`);
-    await delay(delayTime * 1000);
-    return retry(func, { numberOfTries: numberOfTries - 1, secondsDelayRange });
+    const delayTime = Math.floor(Math.random() * (delayRangeMs.max - delayRangeMs.min + 1)) + delayRangeMs.min;


❗Cycode: SAST violation: 'Use of a Broken or Risky Cryptographic Algorithm'.

crypto.pseudoRandomBytes()/Math.random() is a cryptographically weak random number generator.

Severity: Medium

Would you like to exclude this SAST violation from your status checks?
Tell us what to do with one of the following hashtags:

Tag Short Description

#cycode_sast_ignore_here Applies to this request only

#cycode_sast_false_positive Applies to this SAST violation for all repos in your organization

arturwolny-frontegg and others added 14 commits July 31, 2023 10:58

feat(cache): decoupled cache managers from AccessTokens

85db523

build(sdk): npm audit fix

9051f01

fix(cache): Bringing back the ICacheManager generic to the class level

7d04440

refactor(sdk): removed irrelevant accessTokenOptions; refactored cach…

3bbe939

…e manager implementations BREAKING CHANGE: removed accessTokenOptions from FronteggContext configuration

build(sdk): updated lock file

579d601

refactor(sdk): ran lint fix

e30ee78

refactor(sdk): removed irrelevant process warning

756dfb9

docs(sdk): removed examples of Redis cache configuration in access to…

e226bac

…ken section

refactor(sdk): formatted code

fcc357c

test(context): fixed tests of FronteggContext

Merge pull request #154 from frontegg/FR-12681-redis-cache-refactor

1404bfe

refactor(cache): decoupled cache managers from AccessTokens

feat(cache): Adjusted cache to support collections/sets and maps/hash…

4b416f8

…maps

feat(entitlements): Introduced the entitlements cache based of genera…

dfb138e

…l FronteggSDK cache

refactor(entitlements): adjust the entitlements cache with refactored…

83be07d

… frontegg cache component

arturwolny-frontegg force-pushed the FR-12682-redis-entitlements-cache branch from df5dcfc to 83be07d Compare August 1, 2023 09:02

arturwolny-frontegg added 2 commits August 1, 2023 12:10

refactor(sdk): reformatting applied

0eec25e

ci(sdk): fixed tests in CI

3ec0c67

arturwolny-frontegg marked this pull request as ready for review August 14, 2023 09:25

arturwolny-frontegg requested a review from eran-frontegg as a code owner August 14, 2023 09:25

arturwolny-frontegg requested a review from yossi-frontegg August 16, 2023 08:04

arturwolny-frontegg commented Aug 16, 2023

View reviewed changes

arturwolny-frontegg force-pushed the FR-12682-redis-entitlements-cache branch from 6e12d8e to 7a0548e Compare August 16, 2023 12:28

feature(entitlements): leader election for entitlements client cache …

5c3bbf3

…update

arturwolny-frontegg force-pushed the FR-12682-redis-entitlements-cache branch from 7a0548e to 5c3bbf3 Compare August 16, 2023 12:35

yossi-frontegg reviewed Aug 17, 2023

View reviewed changes

feature(entitlements): CR fixes

3ec695b

arturwolny-frontegg force-pushed the FR-12682-redis-entitlements-cache branch from 5b22c42 to 3ec695b Compare August 28, 2023 13:04

arturwolny-frontegg requested a review from yossi-frontegg August 28, 2023 13:16

eran-frontegg force-pushed the next branch from 5b17898 to 4f43917 Compare October 30, 2023 09:23

cycode-security bot reviewed Mar 17, 2024

View reviewed changes


		constructor(private readonly cache: ICacheManager<CacheValue>) {}

		async loadSnapshotAsCurrent(dto: VendorEntitlementsDto): Promise<IsUpdatedToRev> {


		const cacheInitializer = new FronteggEntitlementsCacheInitializer(entitlementsCache);

		const sources = new DtoToCacheSourcesMapper().map(dto);

Tag	Short Description
#cycode_sast_ignore_here	Applies to this request only
#cycode_sast_false_positive	Applies to this SAST violation for all repos in your organization

Conversation

arturwolny-frontegg commented Jul 31, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

arturwolny-frontegg Aug 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

arturwolny-frontegg Aug 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cycode-security bot Mar 17, 2024

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

arturwolny-frontegg Aug 28, 2023 •

edited

Loading

arturwolny-frontegg Aug 28, 2023 •

edited

Loading