Skip to content
This repository was archived by the owner on May 3, 2024. It is now read-only.

Remove express wrapper #288

Merged
salman90 merged 4 commits into from
May 15, 2023
Merged

Remove express wrapper #288

salman90 merged 4 commits into from
May 15, 2023

Conversation

@salman90
Copy link
Contributor

@salman90 salman90 commented May 9, 2023

Purpose

  • Remove express wrapper from sample.

Does this introduce a breaking change?

[ x ] Yes
[ ] No

Pull Request Type

What kind of change does this Pull Request introduce?

[ ] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Documentation content changes
[ ] Other... Please describe:

How to Test

  • Get the code
git clone [repo-address]
cd [repo-name]
git checkout [branch-name]
npm install
  • Test the code

What to Check

Verify that the following are valid

  • ...

Other Information

@salman90 salman90 marked this pull request as draft May 9, 2023 00:14
github-advanced-security[bot]
github-advanced-security bot found potential problems May 9, 2023
View reviewed changes
6-AdvancedScenarios/3-call-api-acrs/API/routes/adminRoutes.js
router.get(
'/dashboard',
isAuthenticated,
authProvider.getToken(protectedResources.msGraphAcrs.scopes),

Check failure

Code scanning / CodeQL

Missing rate limiting

This route handler performs [authorization](1), but is not rate-limited.
6-AdvancedScenarios/3-call-api-acrs/API/routes/adminRoutes.js
authProvider.isAuthenticated(),
dashboardController.postDetailsPage
);
router.post('/details', isAuthenticated, dashboardController.postDetailsPage);

Check failure

Code scanning / CodeQL

Missing rate limiting

This route handler performs [authorization](1), but is not rate-limited.
6-AdvancedScenarios/3-call-api-acrs/API/routes/adminRoutes.js
authProvider.isAuthenticated(),
dashboardController.deleteDetailsPage
);
router.delete('/details', isAuthenticated, dashboardController.deleteDetailsPage);

Check failure

Code scanning / CodeQL

Missing rate limiting

This route handler performs [authorization](1), but is not rate-limited.
6-AdvancedScenarios/3-call-api-acrs/API/authConfig.js
};

const API_REQUIRED_PERMISSION = process.env.API_REQUIRED_PERMISSION || "access_as_user";
const EXPRESS_SESSION_SECRET = process.env.EXPRESS_SESSION_SECRET || 'ENTER_YOUR_SECRET_HERE';

Check failure

Code scanning / CodeQL

Hard-coded credentials

The hard-coded value "ENTER_YOUR_SECRET_HERE" is used as [key](1).
@salman90 salman90 marked this pull request as ready for review May 9, 2023 22:04
@salman90 salman90 requested a review from derisen May 11, 2023 16:55
derisen
derisen reviewed May 12, 2023
View reviewed changes
derisen
derisen reviewed May 12, 2023
View reviewed changes
derisen
derisen reviewed May 12, 2023
View reviewed changes
derisen
derisen approved these changes May 12, 2023
View reviewed changes
Copy link
Contributor

@derisen derisen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks @salman90 -couple comments to address

derisen
derisen approved these changes May 12, 2023
View reviewed changes
Copy link
Contributor

@derisen derisen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks @salman90 -couple comments to address

github-advanced-security[bot]
github-advanced-security bot found potential problems May 15, 2023
View reviewed changes
6-AdvancedScenarios/3-call-api-acrs/API/routes/adminRoutes.js
router.get('/signin', authProvider.signIn({ postLoginRedirect: '/admin' }));
router.get('/signout', authProvider.signOut({ postLogoutRedirect: '/admin' }));
// authentication routes
router.get('/signin', authProvider.login);

Check failure

Code scanning / CodeQL

Missing rate limiting

This route handler performs [authorization](1), but is not rate-limited.
@salman90 salman90 merged commit 8ca0fb0 into main May 15, 2023
@derisen derisen deleted the update-acrs-sample branch June 25, 2023 03:53
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@derisen derisen derisen approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@salman90 @derisen