Skip to content

Conversation

@timmydoza
Copy link
Contributor

@timmydoza timmydoza commented Nov 6, 2019

Enable app to be deployed in Google's app engine. Update circle-ci config to deploy automatically.

Contributing to Twilio

All third party contributors acknowledge that any contributions they provide will be made under the same open source license that the open source project is provided under.

  • I acknowledge that all my contributions will be made under the project's license.

Pull Request Details

JIRA link(s):

Description

This PR adds files needed for deployment to Google's app engine. The CircleCI config has been updated to automatically deploy new tags.

Burndown

Before review

  • Updated CHANGELOG.md if necessary
  • Added unit tests if necessary
  • Updated affected documentation
  • Verified locally with npm test
  • Manually sanity tested running locally
  • Included screenshot as PR comment (if needed)
  • Ready for review

Before merge

  • Got one or more +1s
  • Re-tested if necessary

@timmydoza timmydoza self-assigned this Nov 6, 2019
Copy link
Collaborator

@charliesantos charliesantos left a comment

Choose a reason for hiding this comment

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

This looks good. But since this is now a public url, someone can easily grab our token and use it, which is something we and security wouldn't want. Since we don't have SSO, let's implement Basic Authentication at minimum. Please add the snippet below in your server file. Let's talk about the credentials in slack.

app.use((req, res, next) => {
  // REMOVE THIS! Must be read from environment variable
  const USER_NAME = "myuser";
  const PASSWORD = "password";

  // Get creds from headers
  const b64auth = (req.headers.authorization || '').split(' ')[1] || '';
  const [ login, password ] = new Buffer(b64auth, 'base64').toString().split(':');

  // If authenticated
  if (login && password && login === USER_NAME && password === PASSWORD) {
    return next();
  }

  // Not authenticated, ask for creds
  res.set('WWW-Authenticate', 'Basic realm="Restricted"');
  res.status(401).send('Authentication required.');
});

@timmydoza
Copy link
Contributor Author

timmydoza commented Nov 7, 2019

Good call. For convenience, do you think we should set a cookie too? Otherwise we would have to enter login information every time.

app.use(cookieParser());
app.use((req, res, next) => {
  // REMOVE THIS! Must be read from environment variable
  const USER_NAME = 'myuser';
  const PASSWORD = 'password';

  // Get creds from headers
  const b64auth = (req.headers.authorization || '').split(' ')[1] || '';
  const [login, password] = new Buffer(b64auth, 'base64').toString().split(':');
  const isPasswordValid = login && password && login === USER_NAME && password === PASSWORD;
  const isCookieValid = req.cookies['authCookie'] === 'cookie-secret'; // from env variable

  // If authenticated
  if (isCookieValid || isPasswordValid) {
    passwordIsValid && res.cookie('authCookie', secret, { expires: new Date(Date.now() + 900000) });
    return next();
  }

  // Not authenticated, ask for creds
  res.set('WWW-Authenticate', 'Basic realm="Restricted"');
  res.status(401).send('Authentication required.');
});

I may also add logic so that this is disabled when run locally.

@charliesantos
Copy link
Collaborator

We should not set a different secret to your cookies. A better way is to grab the base64 authorization header and save it to your cookies if it matches the credentials. Then you can check for the base64 auth string in either headers or cookies

@timmydoza timmydoza merged commit 3f6709a into development Nov 7, 2019
@timmydoza timmydoza deleted the AHOYAPPS-97-RC-delivery-process branch November 7, 2019 20:03
timmydoza pushed a commit that referenced this pull request Mar 6, 2020
* Add app.yaml file for google app engine deployment

* Add .gcloudignore

* Update config.yaml to add deployment step

* Change server port to 8080

* Fix circleci config

* Change circleci config

* Use CIRCLE_TAG variable

* Add basic auth to token server

* Import cookieParser

* Remove cookies from server
timmydoza pushed a commit that referenced this pull request Sep 30, 2020
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.

3 participants