Backend add security check if user already accept the terms

[MCatherine1994]

Describe the task Add security check to backend apis that will be used by BCeID delegated admin, to check if the user already accept the terms and conditions

Acceptance Criteria

• [ ] Update backend apis (the ones bceid delegated admin will use), check if user already accept the terms and conditions
• [ ] Add or fix tests

Additional context

• split from #1382
• Or here

[ianliuwk1019]

For apis to check for (BCeID) T&C the check will need to know which "version" of T&C the api requesting user currently accepts. Last time during discussion it seems to me the T&C will be saved at frontend; However, if we want every related endpoint to pass the "version" from frontend, it requires to include the T&C version as new parameter in request for every related endpoints. It will have a broader impact on these endpoints to adjust. Do we want that?

Or, if we don't save the T&C (and its version at backend), we could define a T&C version in a "business constant" (in code, not in db), and that will be the most recent "version" to rely on. However, the drawback (small) is when there is T&C update at frontend, we also need to update this "constant" at the backend. Is it acceptable for this design question? @basilv @MCatherine1994

[basilv]

The backend can hardcode the current T&C version and check if the version the user accepted is the current version.

[ianliuwk1019]

Hi @basilv , as discussed, for now I can enforce the T&C validation only on POST/PUT endpoints and temporarily ignore the GET (and the search [on user/client-number] endpoints.

For your concerns on every endpoint that (if applicable) goes extra database trip(s) to validate the T&C, you are correct, it does go extra trip(or trips) to validate T&C. (For a search, or multiple search, or predictive search, they are already very slow to the idim service, and we probably don't want extra 1 or 2 db session validation checks again each time)

[ Current implementation for validating user on T&C is: checks if user is "external (BCeID) delegated admin" (that's 1 db trip to fam_access_control_privilege table), if yes, then checks if user has T&C record (that's 2nd db trip to fam_user_terms_conditions table) ]

However, I can see it will depend on the nature of the endpoint, that you may or may not find that impose significant performance hit; The reason is, if the endpoint that requires several auth (router guards) checks before executing controller endpoint logic, then it already goes to database for several essential checks, and very likely we could improve on these checks that for FastAPI (our backend Python framework) to cache these database trip results and reuse it (but not every endpoint has the same checks requirement). But at the best, if the endpoints need, still need at least 1 db trip.

It seems to me, still, the validation can be more performant to just find out the user's T&C state at the very beginning when the endpoint retrieve the user information for auth validation from db (1 db trip). At that time, doing a joined query (fam_user join fam_access_control_privilege and fam_user_terms_conditions ) is much better speed than needing to establish another (1 or 2 ) db sessions and do a select queries separately on fam_access_control_privilege and fam_user_terms_conditions tables. The rest, is just some if condition if the endpoints need to check on T&C, and is very quick. (But I don't know from the perceptive if we have 100000... users with join statement for a specific user, maybe the good db index help?) Unless we also want to make frontend performant (by adding the T&C state into get_my_admin_access return response) I think we don't need to duplicate code into the other project workspace ("admin_management")

[basilv]

If we don't want to refactor the router guards now, I'd say maybe raise a ticket to optimize it later. But yes, doing an initial joined query to get all the info needed would be most likely way faster. We just need to ensure foreign keys are indexed on the tables, that will mostly mitigate performance issues.