feat(server, web): mock auth server for local #1135
Conversation
WalkthroughThe changes in this pull request introduce mock authentication capabilities across both server and web components. New environment variables are added for mock authentication in configuration files. The Makefile is updated to include a target for creating a mock user. A new test file validates the mock authentication functionality. The server's context management and middleware are enhanced to support mock authentication, while the web application introduces a new authentication provider and a custom hook for managing mock authentication state. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant WebApp
participant Server
participant AuthProvider
User->>WebApp: Initiate login
WebApp->>AuthProvider: Check auth provider
alt Mock Auth
AuthProvider->>WebApp: Render MockWrapper
WebApp->>AuthProvider: Call useMockAuth
AuthProvider->>Server: Simulate login
Server-->>AuthProvider: Return mock token
else Real Auth
AuthProvider->>Server: Call real authentication
Server-->>AuthProvider: Return real token
end
AuthProvider-->>WebApp: Return auth state
WebApp-->>User: Display login result
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
✅ Deploy Preview for reearth-web ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
rot1024
force-pushed
the
main
branch
2 times, most recently
from
79d81b0
to
a40485c
Compare
hexaforce
force-pushed
the
feat/mock-auth-server-for-local
branch
from
dd4bde3
to
b6c5b2b
Compare
hexaforce
force-pushed
the
feat/mock-auth-server-for-local
branch
from
dbecba6
to
5fcf77c
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1135 +/- ##
==========================================
+ Coverage 13.85% 17.31% +3.46%
==========================================
Files 595 584 -11
Lines 64056 63149 -907
Branches 660 657 -3
==========================================
+ Hits 8873 10935 +2062
+ Misses 55183 52214 -2969
Flags with carried forward coverage won't be shown. Click here to find out more.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Outside diff range and nitpick comments (15)
server/Makefile (1)
42-43: LGTM with suggestions: 'mockuser' target implemented correctly.The new 'mockuser' target is implemented correctly and aligns with the PR objectives. It provides a convenient way to create a mock user for local development. However, consider the following suggestions for improvement:
- Add error handling to provide feedback if the curl command fails.
- Capture and display the server's response to confirm successful user creation.
- Consider making the mock user's email and username configurable through environment variables.
Here's an example of how you might implement these suggestions:
mockuser: @response=$$(curl -s -w "\n%{http_code}" -H 'Content-Type: application/json' \ -d '{"email": "$$MOCK_USER_EMAIL", "username": "$$MOCK_USER_NAME"}' \ http://localhost:8080/api/signup); \ body=$$(echo "$$response" | sed -e '$$d'); \ status=$$(echo "$$response" | tail -n1); \ if [ $$status -eq 200 ]; then \ echo "Mock user created successfully. Response:"; \ echo "$$body"; \ else \ echo "Failed to create mock user. Status code: $$status"; \ echo "Response body:"; \ echo "$$body"; \ exit 1; \ fiThis implementation uses environment variables
MOCK_USER_EMAILandMOCK_USER_NAMEfor configurability, captures the response, and provides appropriate feedback.web/src/services/auth/mockAuth.ts (4)
10-16: LGTM: Initial loading simulation is well-implemented.The
useEffecthook simulates an initial loading state, providing a realistic feel for the mock authentication. The cleanup function prevents potential memory leaks, which is a good practice.Consider extracting the delay duration (1000ms) into a named constant at the top of the file for better maintainability and consistency with other timeouts in the code.
18-25: LGTM: Login function is well-implemented, but consider adding error simulation.The
loginfunction is correctly implemented usinguseCallbackfor performance optimization. It simulates a login process with a realistic delay and updates all relevant states correctly.Consider adding a mechanism to simulate login failures. This could involve a random chance of setting an error state instead of authenticating, which would make the mock more robust for testing error handling in the main application.
Example implementation:
const login = useCallback(() => { setIsLoading(true); setTimeout(() => { if (Math.random() < 0.1) { // 10% chance of login failure setError("Simulated login failure"); setIsAuthenticated(false); } else { setIsAuthenticated(true); setError(null); } setIsLoading(false); }, 1000); }, []);
36-51: LGTM: getAccessToken function is well-implemented, but consider adding error simulation.The
getAccessTokenfunction is correctly implemented usinguseCallbackwith the appropriate dependency. It simulates token retrieval for both authenticated and unauthenticated states, ensuring a unique token each time. The function also correctly updates the authentication state if the user wasn't previously authenticated.Consider adding a mechanism to simulate token retrieval failures. This could involve a random chance of rejecting the promise, which would make the mock more robust for testing error handling in the main application.
Example implementation:
const getAccessToken = useCallback(() => { return new Promise<string>((resolve, reject) => { if (Math.random() < 0.1) { // 10% chance of token retrieval failure setTimeout(() => { reject(new Error("Simulated token retrieval failure")); }, 100); } else if (isAuthenticated) { // ... rest of the existing code for authenticated state } else { // ... rest of the existing code for unauthenticated state } }); }, [isAuthenticated]);
1-61: Overall assessment: Well-implemented mock authentication system with room for minor enhancements.This implementation of a mock authentication system using React hooks is comprehensive and well-structured. It effectively covers all basic authentication functionalities (login, logout, and token retrieval) while providing a realistic feel with simulated delays.
Key strengths:
- Effective use of React hooks (useState, useEffect, useCallback).
- Clear separation of concerns with distinct functions for each authentication action.
- Realistic simulation of asynchronous operations.
Recommendations for enhancement:
- Extract delay durations into named constants for better maintainability.
- Implement error simulation in login and token retrieval functions to make the mock more robust for testing error handling scenarios.
- Consider adding JSDoc comments to functions and the main hook for better documentation.
These enhancements would further improve the mock's utility in development and testing scenarios.
server/e2e/mock_test.go (4)
12-20: LGTM: Test function declaration and server setup are well-structured.The test function is appropriately named and the server configuration is set up correctly for mock authentication testing.
Consider adding a brief comment explaining the purpose of this test function for better documentation.
22-36: LGTM: User signup simulation is well-implemented.The user signup simulation and response validation are correctly implemented. Extracting the user ID for later use is a good practice.
Consider using constants for the test email and username to improve maintainability.
44-57: LGTM: GraphQL query execution and response validation are thorough.The GraphQL query execution is correctly implemented with appropriate headers. The response validation is thorough, checking for the presence of user data and verifying the values.
Consider extracting the response validation into a separate helper function to improve readability and reusability. For example:
func validateUserResponse(t *testing.T, response *httpexpect.Object, expectedName, expectedEmail string) { response.Value("data").Object().Value("me").Object().ContainsKey("id") response.Value("data").Object().Value("me").Object().Value("name").String().Equal(expectedName) response.Value("data").Object().Value("me").Object().Value("email").String().Equal(expectedEmail) }Then you can call this function in your test:
validateUserResponse(t, response2, "Mock User", "[email protected]")
1-57: Overall, excellent implementation of the mock authentication end-to-end test.This test file provides a comprehensive end-to-end test for the mock authentication feature. It covers both user signup and data retrieval, effectively validating the mock authentication flow. The test structure is clear, well-organized, and easy to follow.
To further enhance the test coverage, consider adding the following:
- A negative test case to ensure that the mock authentication fails when the
REEARTH_MOCKAUTHflag is set to false.- A test case to verify that the mock authentication only works for allowed origins.
These additions would help ensure the robustness of the mock authentication implementation across different scenarios.
server/.env.example (1)
26-28: LGTM! Consider adding a comment for clarity.The addition of the
REEARTH_MOCKAUTHenvironment variable is appropriate for enabling mock authentication in local development. Its placement in a new "Local Mock Auth" section is logical and consistent with the file's structure.To improve clarity, consider adding a brief comment explaining the purpose and expected values for this variable. For example:
# Local Mock Auth -REEARTH_MOCKAUTH= +# Set to 'true' to enable mock authentication for local development +REEARTH_MOCKAUTH=This addition would help developers understand how to use this configuration option without referring to external documentation.
server/internal/adapter/http/user.go (1)
94-94: LGTM: Mock auth parameter added correctly.The addition of the
MockAuthparameter usingadapter.IsMockAuth(ctx)is implemented correctly and aligns with the PR objectives. This change allows theSignupmethod to handle mock authentication scenarios without affecting its signature.Consider adding error handling or logging for cases where mock auth might fail or be misconfigured. This could help with debugging in the future. For example:
mockAuth, err := adapter.IsMockAuth(ctx) if err != nil { // Log the error or handle it appropriately log.Printf("Error determining mock auth status: %v", err) // Decide whether to proceed with mockAuth as false or return an error }This suggestion assumes that
IsMockAuthmight return an error. If it doesn't, you can ignore this part of the comment.server/internal/app/config/config.go (3)
184-193: LGTM with suggestion: Consider parameterizing mock values.The changes to
JWTProviders()correctly implement mock authentication support. However, consider parameterizing the mock values (issuer, audience, algorithm, TTL) instead of hardcoding them. This would allow for more flexibility in different development scenarios.For example:
func (c *Config) MockJWTProvider() appx.JWTProvider { return appx.JWTProvider{ ISS: c.MockAuth_ISS, AUD: c.MockAuth_AUD, ALG: c.MockAuth_ALG, TTL: c.MockAuth_TTL, } }Then update the
Configstruct and theJWTProviders()method accordingly.
198-206: LGTM: Consider applying the same parameterization suggestion.The changes to
AuthForWeb()correctly implement mock authentication support. As suggested in theJWTProviders()method review, consider parameterizing the mock values for better flexibility.If you implement the suggested
MockJWTProvider()method, you could simplify this to:if c.UseMockAuth() { provider := c.MockJWTProvider() return &AuthConfig{ ISS: provider.ISS, AUD: provider.AUD, ALG: provider.ALG, TTL: provider.TTL, } }
252-258: LGTM with suggestion: Consider moving utility functions.The
strPtrandintPtrutility functions are well-implemented and serve their purpose in creating mock configurations. However, if these functions are or might be used across multiple packages, consider moving them to a separate utility package for better code organization and reusability.For example:
package utils func StrPtr(s string) *string { return &s } func IntPtr(i int) *int { return &i }Then import and use these functions as needed.
server/internal/adapter/context.go (1)
21-21: Consider standardizingContextKeyconstant namingThe
ContextKeyconstants have inconsistent naming conventions (e.g.,contextUser,ContextAuthInfo,contextUsecases). For better consistency and readability, consider using a uniform naming style for allContextKeyconstants.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
⛔ Files ignored due to path filters (1)
server/go.sumis excluded by!**/*.sum
📒 Files selected for processing (12)
- server/.env.example (1 hunks)
- server/Makefile (2 hunks)
- server/e2e/mock_test.go (1 hunks)
- server/go.mod (1 hunks)
- server/internal/adapter/context.go (4 hunks)
- server/internal/adapter/http/user.go (2 hunks)
- server/internal/app/app.go (1 hunks)
- server/internal/app/auth_client.go (1 hunks)
- server/internal/app/config/config.go (4 hunks)
- web/.env.example (1 hunks)
- web/src/services/auth/authProvider.tsx (2 hunks)
- web/src/services/auth/mockAuth.ts (1 hunks)
✅ Files skipped from review due to trivial changes (2)
- server/go.mod
- web/.env.example
🧰 Additional context used
🔇 Additional comments (21)
server/Makefile (3)
16-16: LGTM: Clear and consistent help message added.The new help message for the 'mockuser' target is well-written and consistent with the existing help messages. It clearly describes the purpose of the new target.
45-45: LGTM: Correct update to .PHONY declaration.The 'mockuser' target has been correctly added to the .PHONY declaration. This is in line with Makefile best practices and ensures that the target will always execute, regardless of the presence of a file named 'mockuser'.
Line range hint
1-45: Overall assessment: Changes align with PR objectives and are well-implemented.The modifications to the Makefile successfully introduce the capability to create a mock user for local development, aligning with the PR's objective to implement mock authentication. The changes are consistent with the existing Makefile structure and follow best practices.
Key points:
- The new 'mockuser' target is correctly implemented and documented.
- The .PHONY declaration is properly updated.
- The help message is clear and consistent.
While the implementation is sound, consider the suggested improvements to the 'mockuser' target for enhanced usability and robustness.
web/src/services/auth/mockAuth.ts (4)
1-3: LGTM: Imports and type definitions are appropriate.The necessary React hooks are imported, and the
AuthHooktype is imported from a local file, which is likely defining the structure of the hook's return value. This setup provides good type safety and modularity.
5-8: LGTM: State management is well-structured.The hook initializes three state variables (
isAuthenticated,isLoading, anderror) which comprehensively cover the authentication status. This separation allows for fine-grained control and updates, enhancing the flexibility of the mock authentication system.
27-34: LGTM: Logout function is well-implemented.The
logoutfunction is correctly implemented usinguseCallbackfor performance optimization. It simulates a logout process with a realistic delay (appropriately shorter than the login delay) and updates all relevant states correctly.
53-61: LGTM: Return statement is complete and well-structured.The hook returns an object containing all necessary properties and methods for authentication. This structure appears to be consistent with the imported
AuthHooktype, ensuring type safety and providing a complete interface for the authentication system.server/e2e/mock_test.go (2)
1-11: LGTM: Package declaration and imports are appropriate.The package declaration, imports, and the comment for running the test are all correct and helpful.
38-43: LGTM: GraphQL query setup is correct and well-structured.The GraphQL query for retrieving user information is correctly structured. Using a struct for the request body is a good practice that enhances readability and maintainability.
web/src/services/auth/authProvider.tsx (4)
12-12: LGTM: New import for mock authentication.The import for
useMockAuthis correctly placed and follows the established naming convention. This aligns well with the PR objective of implementing mock authentication for local development.
26-31: LGTM: MockWrapper component implementation.The
MockWrappercomponent is well-implemented and consistent with the existing wrapper components. It correctly utilizes theuseMockAuthhook and provides the mock auth context to its children.
Line range hint
1-71: Overall implementation of mock authentication looks good, with one point to address.The changes in this file successfully implement the mock authentication provider as outlined in the PR objectives. The new
MockWrappercomponent and the modifications to theAuthProviderare well-structured and consistent with the existing code.However, please address the following point:
- Verify the necessity of the
logInToTenant()call for mock authentication, and consider adding conditional logic to skip it when using mock auth if it's not required.Once this is addressed, the implementation will be robust and fully aligned with the PR objectives.
37-43: Verify the necessity oflogInToTenant()for mock authentication.The changes to support mock authentication in the
AuthProvidercomponent look good. However, please verify if thelogInToTenant()function call is necessary for mock authentication. If not, consider wrapping it in a condition to skip it when using mock authentication.To verify the usage and necessity of
logInToTenant(), run the following script:server/.env.example (1)
Line range hint
1-28: Overall changes look good and maintain file integrity.The addition of the
REEARTH_MOCKAUTHvariable in the new "Local Mock Auth" section is well-placed and consistent with the file's existing structure. The change doesn't disrupt the overall organization of the configuration file and maintains the established format.This change effectively introduces the capability for mock authentication in local development environments without affecting other configuration options. Ensure that the usage of this new variable is well-documented in the project's setup instructions or developer guidelines.
server/internal/adapter/http/user.go (1)
6-6: LGTM: New import statement is appropriate.The addition of the
adapterpackage import is necessary and correctly placed for the subsequent use ofadapter.IsMockAuth.server/internal/app/config/config.go (3)
73-73: LGTM: MockAuth field added correctly.The
MockAuthboolean field is a good addition to theConfigstruct. It aligns with the PR objectives and follows the existing coding style.
129-131: LGTM: UseMockAuth method implemented correctly.The
UseMockAuth()method is well-implemented. It correctly checks both theDevandMockAuthflags, ensuring that mock authentication is only used in development environments. This aligns perfectly with the PR objectives.
Line range hint
1-258: Overall assessment: Well-implemented mock authentication with room for minor improvements.The changes to
server/internal/app/config/config.gosuccessfully implement mock authentication as per the PR objectives. The newMockAuthfield,UseMockAuth()method, and modifications toJWTProviders()andAuthForWeb()methods work together to provide a coherent mock authentication system for development environments.Suggestions for improvement include:
- Parameterizing mock values for better flexibility.
- Considering the relocation of utility functions to a separate package if they have broader applicability.
These suggestions aim to enhance the maintainability and flexibility of the code without changing its core functionality.
server/internal/adapter/context.go (2)
48-50:AttachMockAuthfunction is correctly implementedThe
AttachMockAuthfunction properly attaches themockAuthboolean value to the context, enabling mock authentication when required.
117-124:IsMockAuthfunction correctly retrieves mock authentication statusThe
IsMockAuthfunction accurately checks the context for themockAuthvalue and returns the appropriate boolean status.server/internal/app/app.go (1)
77-78: Verify the middleware order to ensure expected behaviorThe
attachOpMiddleware(cfg)is now applied after the authentication middleware. Ensure that this change doesn't introduce any unintended side effects, especially ifattachOpMiddlewarerelies on context set by earlier middleware. Middleware order can affect request handling and authentication flow.To verify whether
attachOpMiddlewaredepends on authentication context, you can run the following script:This script searches for the
attachOpMiddlewarefunction and checks if it references authentication-related context.✅ Verification successful
Middleware order verified and no issues found.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if attachOpMiddleware depends on authentication context # Search for occurrences where attachOpMiddleware accesses authentication information rg --type go 'func attachOpMiddleware' -A 15 | rg 'ContextAuthInfo|authInfo'Length of output: 78
Script:
#!/bin/bash # Description: Comprehensive search for authentication context usage within attachOpMiddleware # Search for any authentication-related context access within the attachOpMiddleware function rg --type go 'func attachOpMiddleware' -A 30 | rg 'context|auth|user|token|session'Length of output: 2122
| if IsMockAuth(ctx) { | ||
| return &appx.AuthInfo{ | ||
| Sub: user.NewID().String(), // Use it if there is a Mock user in the DB | ||
| Name: "Mock User", | ||
| Email: "[email protected]", | ||
| } | ||
| } |
There was a problem hiding this comment.
Avoid generating a new user ID for each mock authentication
Generating a new user ID each time GetAuthInfo is called in mock authentication mode can lead to inconsistencies across the application session. This may affect user-specific data retrieval and cache mechanisms that rely on a consistent user ID.
Consider using a fixed mock user ID or retrieving the mock user from the database to ensure consistency. Apply the following change:
func GetAuthInfo(ctx context.Context) *appx.AuthInfo {
if IsMockAuth(ctx) {
return &appx.AuthInfo{
- Sub: user.NewID().String(), // Use it if there is a Mock user in the DB
+ Sub: "mock-user-id", // Use a fixed mock user ID
Name: "Mock User",
Email: "[email protected]",
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if IsMockAuth(ctx) { | |
| return &appx.AuthInfo{ | |
| Sub: user.NewID().String(), // Use it if there is a Mock user in the DB | |
| Name: "Mock User", | |
| Email: "[email protected]", | |
| } | |
| } | |
| if IsMockAuth(ctx) { | |
| return &appx.AuthInfo{ | |
| Sub: "mock-user-id", // Use a fixed mock user ID | |
| Name: "Mock User", | |
| Email: "[email protected]", | |
| } | |
| } |
There was a problem hiding this comment.
@hexaforce This change makes sense, Can you add it?
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
| if u == nil && au != nil { | ||
| var err error | ||
| // find user | ||
| u, err = multiUser.FetchBySub(ctx, au.Sub) | ||
| if err != nil && err != rerror.ErrNotFound { | ||
| return err | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Handle the case when the user is not found
If the user is not found after attempting to fetch by au.Sub (i.e., u remains nil), consider creating a new user or providing appropriate feedback. This ensures that authenticated users who are new to the system are properly onboarded.
Suggested addition:
if u == nil && au != nil {
// User not found, create a new user
uId, err := user.IDFrom(au.Sub)
if err != nil {
return err
}
u = user.New().
ID(uId).
Name(au.Name).
Email(au.Email).
MustBuild()
// Save the new user to the repository
if err := cfg.AccountRepos.User.Save(ctx, u); err != nil {
return err
}
}| mockUser = user.New(). | ||
| ID(uId). | ||
| Name(au.Name). | ||
| Email(au.Email). | ||
| MustBuild() | ||
| } |
There was a problem hiding this comment.
Persist the new mock user to the repository
After creating a new mock user, the user object is constructed but not saved to the user repository. This means the mock user may not be retrievable in subsequent requests, leading to inconsistencies during authentication.
Suggested code fix:
mockUser = user.New().
ID(uId).
Name(au.Name).
Email(au.Email).
MustBuild()
+ // Save the new mock user to the repository
+ if err := cfg.AccountRepos.User.Save(ctx, mockUser); err != nil {
+ return err
+ }Committable suggestion was skipped due to low confidence.
| mockUser, err := cfg.AccountRepos.User.FindByNameOrEmail(ctx, "Mock User") | ||
| if err != nil { | ||
| // when creating the first mock user | ||
| uId, _ := user.IDFrom(au.Sub) |
There was a problem hiding this comment.
Handle error returned by user.IDFrom(au.Sub)
In line 50, the error returned by user.IDFrom(au.Sub) is being ignored. If au.Sub is invalid, this could lead to unexpected behavior or runtime errors. It's important to handle this error to ensure the robustness of the code.
Suggested code fix:
- uId, _ := user.IDFrom(au.Sub)
+ uId, err := user.IDFrom(au.Sub)
+ if err != nil {
+ // Handle error appropriately
+ return err
+ }Committable suggestion was skipped due to low confidence.
| wrapHandler = lo.Must(appx.AuthMiddleware(authConfig, adapter.ContextAuthInfo, true)) | ||
| } |
There was a problem hiding this comment.
Handle errors from appx.AuthMiddleware without panicking
Using lo.Must will cause a panic if appx.AuthMiddleware returns an error. It's better practice to handle errors gracefully to prevent the entire application from crashing unexpectedly. Consider capturing the error and dealing with it appropriately, such as logging the error and exiting the initialization with a meaningful message.
Here's a suggested change:
wrapHandler = lo.Must(appx.AuthMiddleware(authConfig, adapter.ContextAuthInfo, true))
+// Replace lo.Must with explicit error handling
+authMiddleware, err := appx.AuthMiddleware(authConfig, adapter.ContextAuthInfo, true)
+if err != nil {
+ log.Errorf("Failed to initialize authentication middleware: %v", err)
+ return nil // or handle the error as needed
+}
+wrapHandler = authMiddleware📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| wrapHandler = lo.Must(appx.AuthMiddleware(authConfig, adapter.ContextAuthInfo, true)) | |
| } | |
| authMiddleware, err := appx.AuthMiddleware(authConfig, adapter.ContextAuthInfo, true) | |
| if err != nil { | |
| log.Errorf("Failed to initialize authentication middleware: %v", err) | |
| return nil // or handle the error as needed | |
| } | |
| wrapHandler = authMiddleware | |
| } |
| if cfg.Config.UseMockAuth() { | ||
| log.Infof("Using mock auth for local development") | ||
| wrapHandler = func(next http.Handler) http.Handler { | ||
| return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| ctx := r.Context() | ||
| ctx = adapter.AttachMockAuth(ctx, true) | ||
| next.ServeHTTP(w, r.WithContext(ctx)) | ||
| }) | ||
| } | ||
| } else { |
There was a problem hiding this comment.
Ensure mock authentication is disabled in production environments
The code conditionally enables mock authentication based on the configuration. To prevent accidental use of mock authentication in production, consider adding safeguards to ensure it is only enabled in development environments. This helps mitigate potential security risks where authentication might be bypassed unintentionally.
You might modify the code as follows to include an explicit check:
if cfg.Config.UseMockAuth() {
+ if !cfg.Debug && !cfg.Config.Dev {
+ log.Errorf("Mock authentication cannot be enabled outside of development environments")
+ return nil // or handle the error appropriately
+ }
log.Infof("Using mock auth for local development")
wrapHandler = func(next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
ctx := r.Context()
ctx = adapter.AttachMockAuth(ctx, true)
next.ServeHTTP(w, r.WithContext(ctx))
})
}Committable suggestion was skipped due to low confidence.
Overview
mock auth server for local
What I've done
REEARTH_MOCKAUTH: This will allow the backend to accept requests in mock mode.REEARTH_MOCKAUTH will operate in the development environment, allowing requests to be accepted by skipping JWT token validation.
REEARTH_WEB_AUTH_PROVIDER: Normally set to auth0, but specify mock in mock mode.Modify the backend to create a mock user when REEARTH_MOCKAUTH=true.
Add a mock provider to the AuthContext in the web application. This will be used when REEARTH_WEB_AUTH_PROVIDER=mock.
What I haven't done
How I tested
Which point I want you to review particularly
Memo
Usage
+REEARTH_MOCKAUTH=true$ cd server $ make mockuser+REEARTH_WEB_AUTH_PROVIDER=mockSummary by CodeRabbit
Release Notes
New Features
MockWrappercomponent for conditional rendering in the authentication provider.useMockAuthfor managing mock authentication state.Bug Fixes
Documentation
MockAuthfield for better management of authentication settings.These updates provide developers with improved capabilities for testing and development environments while maintaining existing functionalities.