Skip to content

Commit

Permalink
Merge pull request #2165 from NYPL/NOREF-disallow-internal-hostname
Browse files Browse the repository at this point in the history
Add fix to fix use of discovery.nypl.org in redirect_uri
  • Loading branch information
nonword authored Nov 7, 2024
2 parents 88eb06c + c967489 commit b29a083
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 2 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ jobs:
aws ecs update-service --cluster discovery-ui-edd --service discovery-ui-edd --force-new-deployment
publish_production:
# needs: test
if: github.ref == 'refs/heads/TGR-50/remediation-update'
if: github.ref == 'refs/heads/production'
name: Publish image to ECR and update ECS stack
runs-on: ubuntu-latest
permissions:
Expand Down
4 changes: 3 additions & 1 deletion src/server/ApiRoutes/User.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@ function requireUser(req, res) {
const originalUrl = req.originalUrl.replace(new RegExp(`^${appConfig.baseUrl}/api/`), `${appConfig.baseUrl}/`)
// TODO: Express 4.x strips the port from req.hostname, inconveniencing
// local development. May cautiously retrieve it from headers or local config
const fullUrl = encodeURIComponent(`${req.protocol}://${req.hostname}${originalUrl}`);
const fullUrl = encodeURIComponent(`${req.protocol}://${req.hostname}${originalUrl}`)
// Don't allow internal hostname ([qa-]discovery.nypl.org) to be used in redirect:
.replace('discovery.nypl.org', 'www.nypl.org');
redirect = `${appConfig.loginUrl}?redirect_uri=${fullUrl}`;
if (!req.originalUrl.includes('/api/')) {
res.redirect(redirect);
Expand Down

0 comments on commit b29a083

Please sign in to comment.