Feat/add jwt to authjs #215

SEBRATHEZEBRA · 2023-08-25T08:17:54Z

No description provided.

github-actions · 2023-08-25T08:21:47Z

Risk Level 2 - /home/runner/work/code-review-gpt/code-review-gpt/services/web-app/src/app/profile/page.tsx

The changes in the profile page have a moderate risk to the code base. The added code retrieves the user data from the server using the 'getUser' endpoint and displays it on the page. No potential bugs or readability issues were found. However, it would be better to handle the case when the user data cannot be retrieved separately from the case when the user is not logged in.

Risk Level 3 - /home/runner/work/code-review-gpt/code-review-gpt/services/web-app/src/lib/hooks/useAxios.tsx

Potential bug: The getSession function is called without any arguments. This may cause an error or unexpected behavior. Please ensure that the getSession function is called correctly.
Readability improvement: The code can be made more readable by providing a type annotation for the session variable.
SOLID principle: The code violates the Single Responsibility Principle (SRP) by combining the logic of getting the session and setting the authorization header in the same function. It would be better to separate these concerns into separate functions or modules.

Example code snippet for readability improvement:

const session: Session | null = getSession();

Example code snippet for separating concerns:

const setAuthorizationHeader = (config: AxiosRequestConfig, session: Session) => {
  config.headers.Authorization = session.token;
  return config;
};

axiosInstance.interceptors.request.clear();
axiosInstance.interceptors.request.use((config) => setAuthorizationHeader(config, session));

Risk Level 3 - /home/runner/work/code-review-gpt/code-review-gpt/services/core/functions/update-user/index.ts

The changes in the update-user function have a moderate risk to the code base. The added code retrieves the 'userId' from the request body and uses it to update the user in the UserEntity. No potential bugs or readability issues were found. However, it would be better to handle the case when 'apiKey' or 'userId' is not provided separately from the error when updating the user.

🔍🔧👍

Powered by Code Review GPT

github-actions · 2023-08-25T08:22:39Z

Test results summary:

✅ [PASS] - Test case: Bad variable name
❌ [FAIL] - Test case: Exposed secret
✅ [PASS] - Test case: Too many nested loops
⚠️ [WARN] - Test case: Unawaited Promise

SUMMARY: ✅ PASS: 2 - ⚠️ WARN: 1 - ❌ FAIL: 1

Tests Powered by Code Review GPT

lizacullis

Can you also please check the Update-User and make sure that that also has the email changed to userId.

lizacullis · 2023-08-29T15:59:07Z

services/core/functions/get-user/index.ts


-    if (email === undefined) {
+    if (userId === undefined) {
      return formatResponse("Please provide the email of the user you wish to get.", 400)


Suggested change

return formatResponse("Please provide the email of the user you wish to get.", 400)

return formatResponse("Please provide the userId of the user you wish to get.", 400)

lizacullis · 2023-08-29T16:03:02Z

services/core/resources/constructs/review-bucket.ts

-      autoDeleteObjects: !isProduction(),
+      // autoDeleteObjects: !isProduction(),


@mattzcarey Seb and I chatted about this. Adding this autoDeleteObjects thing is a bit broken, and it doesn't allow you to tear down your stack (you will see a couple of stacks already have DeleteFailed). So we have both commented this out for now. Might be a good things to investigate!

Sounds good!

lizacullis · 2023-08-29T16:03:41Z

services/web-app/src/app/profile/page.tsx

-        console.log("Failed to getUser");
+        console.log("Failed to getUser, error -> ", err);


lizacullis · 2023-08-29T16:04:33Z

services/web-app/src/lib/constants.ts

-export const BASE_URL = "https://mylf3hxjs8.execute-api.eu-west-2.amazonaws.com/prod";
+export const BASE_URL = "https://2dw4kjr0a2.execute-api.eu-west-2.amazonaws.com/prod";


We need to chat about how to better do this. Not now's problem, but would be nice if this could be gotten from aws or something!

We will use the staging URL here in future. That is deployed.

mattzcarey · 2023-08-30T10:37:31Z

services/web-app/src/lib/hooks/useAxios.tsx

+  axiosInstance.interceptors.request.clear();
+  axiosInstance.interceptors.request.use(
+    (config) => {
+      config.headers.Authorization = (session as Session).token;


changes done

SEBRATHEZEBRA added 2 commits August 24, 2023 15:04

feat: adding jwt to authjs

cf9558a

feat: add jwt token to axios header

c3af0f1

SEBRATHEZEBRA linked an issue Aug 25, 2023 that may be closed by this pull request

NOT DONE [Auth FE] AASignedInUser I generate a JWT which contains my userId #181

Closed

5 tasks

SEBRATHEZEBRA added 3 commits August 25, 2023 14:31

feat: jwt decoder

796ae25

feat: jwt added to axiosInstance header

b77ffc2

feat: working Profile page with jwt

7bebd19

SEBRATHEZEBRA force-pushed the feat/add-jwt-authjs branch from fcb5151 to 7bebd19 Compare August 29, 2023 10:00

SEBRATHEZEBRA added 3 commits August 29, 2023 12:05

Merge branch 'main' into feat/add-jwt-authjs

5df5dd6

Merge branch 'main' into feat/add-jwt-authjs

26d55a6

feat: changing user unique identifier from email to userId

8deda54

SEBRATHEZEBRA marked this pull request as ready for review August 29, 2023 14:52

SEBRATHEZEBRA added 2 commits August 29, 2023 16:59

Merge branch 'main' into feat/add-jwt-authjs

8a076bd

chore: switch email for userId on Profile page

5f07987

lizacullis previously requested changes Aug 29, 2023

View reviewed changes

chore: requested changes

b1328c7

mattzcarey reviewed Aug 30, 2023

View reviewed changes

mattzcarey approved these changes Aug 30, 2023

View reviewed changes

chore: change UpdateUser lambda to user userId instead of email

3f63286

SEBRATHEZEBRA merged commit b967671 into main Aug 30, 2023

SEBRATHEZEBRA deleted the feat/add-jwt-authjs branch August 30, 2023 10:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat/add jwt to authjs #215

Feat/add jwt to authjs #215

SEBRATHEZEBRA commented Aug 25, 2023

github-actions bot commented Aug 25, 2023 •

edited

Loading

github-actions bot commented Aug 25, 2023 •

edited

Loading

lizacullis left a comment

lizacullis Aug 29, 2023

lizacullis Aug 29, 2023

mattzcarey Aug 30, 2023

lizacullis Aug 29, 2023

lizacullis Aug 29, 2023

mattzcarey Aug 30, 2023 •

edited

Loading

mattzcarey Aug 30, 2023

	return formatResponse("Please provide the email of the user you wish to get.", 400)
	return formatResponse("Please provide the userId of the user you wish to get.", 400)

		autoDeleteObjects: !isProduction(),
		// autoDeleteObjects: !isProduction(),

		console.log("Failed to getUser");
		console.log("Failed to getUser, error -> ", err);

		export const BASE_URL = "https://mylf3hxjs8.execute-api.eu-west-2.amazonaws.com/prod";
		export const BASE_URL = "https://2dw4kjr0a2.execute-api.eu-west-2.amazonaws.com/prod";

Feat/add jwt to authjs #215

Feat/add jwt to authjs #215

Conversation

SEBRATHEZEBRA commented Aug 25, 2023

github-actions bot commented Aug 25, 2023 • edited Loading

Powered by Code Review GPT

github-actions bot commented Aug 25, 2023 • edited Loading

Test results summary:

Tests Powered by Code Review GPT

lizacullis left a comment

Choose a reason for hiding this comment

lizacullis Aug 29, 2023

Choose a reason for hiding this comment

lizacullis Aug 29, 2023

Choose a reason for hiding this comment

mattzcarey Aug 30, 2023

Choose a reason for hiding this comment

lizacullis Aug 29, 2023

Choose a reason for hiding this comment

lizacullis Aug 29, 2023

Choose a reason for hiding this comment

mattzcarey Aug 30, 2023 • edited Loading

Choose a reason for hiding this comment

mattzcarey Aug 30, 2023

Choose a reason for hiding this comment

github-actions bot commented Aug 25, 2023 •

edited

Loading

github-actions bot commented Aug 25, 2023 •

edited

Loading

mattzcarey Aug 30, 2023 •

edited

Loading