Fix/Community-auth-fixes-frontend#1070
Conversation
Fix/merge conflict
Pm release 5
# Conflicts: # backend/src/main/java/com/skapp/community/peopleplanner/service/impl/RolesServiceImpl.java
…ctives Feat/636 company objectives
Develop improvements
Develop improvements
Update README.md
|
|
There was a problem hiding this comment.
Pull request overview
This PR implements authentication fixes for the community edition of the application. It separates community authentication logic from enterprise features by introducing fallback implementations for enterprise-specific functionality and implementing credential-based sign-in/sign-up for the community edition.
Changes:
- Added fallback implementations for enterprise features (auth utilities, enums, API endpoints, calendar hooks, and APICTA dashboard)
- Implemented community-specific sign-in and sign-up functions with proper API endpoints
- Updated authentication flow to conditionally add tenant headers only in enterprise mode
- Fixed sign-in/sign-up status checks to properly access the status property from the response object
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/src/fallback/common/utils/commonUtil.ts | Adds fallback stubs for enterprise utility functions |
| frontend/src/fallback/common/enums/Common.ts | Provides empty enum stubs for enterprise-specific enums |
| frontend/src/fallback/common/api/utils/ApiEndpoints.ts | Adds empty authentication endpoints object for fallback |
| frontend/src/fallback/common/api/CalendarApi.ts | Implements fallback calendar organization status hook |
| frontend/src/fallback/auth/utils/authUtils.ts | Provides stub implementations for enterprise auth functions that always return failure |
| frontend/src/fallback/APICTA/dashboard.tsx | Creates empty APICTA dashboard component for fallback |
| frontend/src/community/common/utils/commonUtil.ts | Adds utility to check if running in enterprise mode |
| frontend/src/community/common/api/utils/ApiEndpoints.ts | Adds new credential-based sign-in/sign-up API endpoints |
| frontend/src/community/auth/utils/authUtils.ts | Implements community sign-in/sign-up functions with proper API integration |
| frontend/src/community/auth/utils/authInterceptor.ts | Conditionally adds tenant header only in enterprise mode |
| frontend/src/community/auth/types/auth.ts | Defines community-specific sign-in/sign-up parameter types |
| frontend/pages/index.tsx | Reorders imports (no functional change) |
| frontend/pages/community/signup.tsx | Fixes status check to access status property from result object |
| frontend/pages/community/signin.tsx | Fixes status check to access status property from result object |
| frontend/middleware.ts | Removes community routes from middleware matcher to allow public access to auth pages |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } catch (error: any) { | ||
| return { | ||
| status: SignInStatus.FAILURE, | ||
| error: error?.response?.data?.[0]?.messageKey | ||
| }; |
There was a problem hiding this comment.
The error handling assumes a specific error response structure error?.response?.data?.[0]?.messageKey, but if the API returns errors in a different format, the messageKey might be undefined. Consider adding a fallback error message or validating the error structure more robustly to prevent returning undefined as the error message.
There was a problem hiding this comment.
same as enterprise
| return false; | ||
| }; | ||
|
|
||
| export const tempShouldUseCustomDashboard = () => { |
There was a problem hiding this comment.
The function signature of tempShouldUseCustomDashboard is inconsistent with its usage. In frontend/pages/community/dashboard/index.tsx line 159, this function is called with a parameter user?.tenantId, but the function doesn't accept any parameters. The function signature should match the expected usage: tempShouldUseCustomDashboard = (tenantId?: string | number) => boolean
| export const tempShouldUseCustomDashboard = () => { | |
| export const tempShouldUseCustomDashboard = (tenantId?: string | number): boolean => { |
There was a problem hiding this comment.
not needed since is a fallback file
| matcher: [ | ||
| // All community routes | ||
| "/community/:path*", | ||
| // Super admin routes | ||
| "/setup-organization/:path*", | ||
| "/module-selection", |
There was a problem hiding this comment.
The removal of the community routes matcher "/community/:path*" from the middleware configuration means that pages under /community/signin and /community/signup will no longer be protected by the middleware. This could be intentional to allow public access to these auth pages, but it's a significant change. If this is intentional, ensure that these pages don't contain any logic that requires middleware protection. If unintentional, this could be a security issue.
There was a problem hiding this comment.
it was done to allow sign in and sign up to not be protected
| } catch (error: any) { | ||
| return { | ||
| status: SignInStatus.FAILURE, | ||
| error: error?.response?.data?.[0]?.messageKey | ||
| }; |
There was a problem hiding this comment.
The error handling assumes a specific error response structure error?.response?.data?.[0]?.messageKey, but if the API returns errors in a different format, the messageKey might be undefined. Consider adding a fallback error message or validating the error structure more robustly to prevent returning undefined as the error message.
There was a problem hiding this comment.
same as enterprise
| } No newline at end of file | ||
| } | ||
| export interface CommunitySignInParams { | ||
| email?: string; |
There was a problem hiding this comment.
same as enterprise
| export interface CommunitySignUpParams { | ||
| firstName?: string; | ||
| lastName?: string; | ||
| email?: string; |
There was a problem hiding this comment.
remove option for required fields
There was a problem hiding this comment.
same as enterprise
| export const authenticationEndpoints = { | ||
| CREATE_SUPER_ADMIN: `${moduleAPIPath.AUTH}/signup/super-admin`, | ||
| SIGN_IN: `${moduleAPIPath.AUTH}/sign-in`, | ||
| CREDENTIAL_SIGN_IN: `${ApiVersions.V1}/auth/session/sign-in`, |
There was a problem hiding this comment.
remove the old end points
| @@ -12,6 +15,8 @@ export const organizationCreateEndpoints = { | |||
| export const authenticationEndpoints = { | |||
| CREATE_SUPER_ADMIN: `${moduleAPIPath.AUTH}/signup/super-admin`, | |||
There was a problem hiding this comment.
check in mobile if not using remove from backend
There was a problem hiding this comment.
we need sign in but create SUPER admin is not needed
There was a problem hiding this comment.
and its already deleted in backend
| @@ -0,0 +1,5 @@ | |||
| const APICTADashboard = () => { | |||
There was a problem hiding this comment.
branch out for future reference and remove all APICTA UIs
remove all the related utils as well
| @@ -0,0 +1,29 @@ | |||
| import { SignInStatus } from "~community/auth/enums/auth"; | |||
There was a problem hiding this comment.
we believe the file can make empty
enterpriseSignIn , enterpriseSignUp only needed with empty body
|
| matcher: [ | ||
| // All community routes | ||
| "/community/:path*", | ||
| // All community routes (excluding public auth pages) |
There was a problem hiding this comment.
lets check what is the rootcause



PR checklist
TaskId: (https://rootcode.skapp.com/pm/projects/SKAPP/items/1381)
Summary
-Community Code auth fixes for frontend
How to test
Project Checklist
npm run formatnpm run check-lintOther
PR Checklist
ready-for-code-review)Additional Information