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

Auto refresh AWS credentials #23

Merged
merged 4 commits into from Dec 29, 2016
Merged

Conversation

@ayumi
Copy link
Contributor

ayumi commented Dec 29, 2016

  • Extract encrypt() and decrypt() from sync.js into cryptoUtil.js
  • Extract refreshAWSCredentials() from sync.js into requestUtil.js
  • requestUtil automatically detects expired credentials and requests new ones from the server
@ayumi ayumi requested a review from diracdeltas Dec 29, 2016
@@ -1,27 +1,103 @@
'use strict'

const MAX_RETRIES = 1
const NONCE_COUNTER = 0

This comment has been minimized.

Copy link
@diracdeltas

diracdeltas Dec 29, 2016

Member

this should be imported from client/config.js so it doesn't need to be edited in more than one place when it changes

credentialsBytes: null, // TODO: Start with previous session's credentials
keys: clientKeys,
serializer: clientSerializer,
syncServerUrl: config.serverUrl

This comment has been minimized.

Copy link
@diracdeltas

diracdeltas Dec 29, 2016

Member

this throws a 'missing serverUrl' error

})
}
const decrypt = cryptoUtil.Decrypt(clientSerializer, clientKeys.secretboxKey)
const encrypt = cryptoUtil.Encrypt(clientSerializer, clientKeys.secretboxKey, conf.nonceCounter)

This comment has been minimized.

Copy link
@diracdeltas

diracdeltas Dec 29, 2016

Member

these functions need to be initialized after clientSerializer is initialized, otherwise clientSerializer is null.

@ayumi ayumi force-pushed the feature/auto-refresh-aws-credentials branch from de7984a to 7182210 Dec 29, 2016
@ayumi ayumi force-pushed the feature/auto-refresh-aws-credentials branch from 7182210 to f420d1f Dec 29, 2016
@ayumi
Copy link
Contributor Author

ayumi commented Dec 29, 2016

@diracdeltas thx, I made some changes based on your feedbacks. could you take another look?

@diracdeltas
Copy link
Member

diracdeltas commented Dec 29, 2016

thanks, lgtm!

@diracdeltas diracdeltas merged commit 6890b53 into staging Dec 29, 2016
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@ayumi ayumi deleted the feature/auto-refresh-aws-credentials branch Dec 29, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.