Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add public api #73

Closed
wants to merge 1 commit into from
Closed

Add public api #73

wants to merge 1 commit into from

Conversation

WilmsJochen
Copy link
Contributor

@WilmsJochen WilmsJochen commented Oct 29, 2024


💡 Summary generated by FirstMate:

  • Added a new public API method in exampleController.js that returns a success message.
  • Updated exampleRouter.js to include a new route for the public API, utilizing the new controller method.

Copy link

firstmatebot bot commented Oct 29, 2024

PR Review

⚠️ It seems that you can still improve the quality of your PR. Have a look into this:

   

❌ Error handling & async: Replace 'next(err)' with 'next(new ApplicationError(err))' to follow error handling guidelines.

These are the improvements you should make:

  • Use ApplicationError for errors: Replace 'next(err)' with 'next(new ApplicationError(err))' to adhere to the guideline.

Use ApplicationError for errors

You are using next(err) in your error handling, but it would be better to use next(new ApplicationError(err)) to ensure that errors are properly formatted and handled according to your application's error management strategy. Here’s how to make the necessary changes in exampleController.js:

-            next(err);
+            next(new ApplicationError(err));

Make sure to apply this change in both the getById and publicApi methods to maintain consistency in error handling.

To improve your PR. Please make changes to these files:

  • /src/controllers/exampleController.js
   
❌ Security & secret management: Add `grantAccessByPermissionMiddleware` to '/public' route for proper permission checks.

These are the improvements you should make:

  • Check permissions with middleware: Add 'grantAccessByPermissionMiddleware' to the '/public' route to ensure proper permission checks.

Check permissions with middleware

You are not enforcing permission checks on the /public route, which is crucial for maintaining security. To ensure proper permission checks, add grantAccessByPermissionMiddleware to the /public route as follows:

router.route("/public")
    .get(grantAccessByPermissionMiddleware([API_PERMISSIONS.PUBLIC_ENDPOINT]), exampleController.publicApi)

This change ensures that only authorized users can access the public API, adhering to the security guidelines.

To improve your PR. Please make changes to these files:

  • /src/routes/exampleRouter.js

Generated by Firstmate to make sure you can focus on coding new features.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant