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.

Sign up for GitHub

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

Jump to bottom

Fix AppKey decryption in Join Server #1642

Merged
merged 2 commits into from
Nov 26, 2019
Merged

Fix AppKey decryption in Join Server #1642

merged 2 commits into from
Nov 26, 2019

Conversation

rvolosatovs
Copy link
Contributor

Summary

Fix-up of #1591

Changes

  • Fix the field name

Notes for Reviewers

Classic copy-paste issue

Checklist

  • Scope: The referenced issue is addressed, there are no unrelated changes.
  • Compatibility: The changes are backwards compatible with existing API, database and configuration, according to the stability commitments in README.md.
  • Testing: The changes are covered with unit tests. The changes are tested manually as well.
  • Documentation: Relevant documentation is added or updated.
  • Changelog: Significant features, behavior changes, deprecations and fixes are added to CHANGELOG.md.
  • Commits: Commit messages follow guidelines in CONTRIBUTING.md, there are no fixup commits left.

@rvolosatovs rvolosatovs added the bug Something isn't working label Nov 26, 2019
@rvolosatovs rvolosatovs added this to the November 2019 milestone Nov 26, 2019
@rvolosatovs rvolosatovs self-assigned this Nov 26, 2019
@rvolosatovs rvolosatovs removed the request for review from htdvisser November 26, 2019 09:27
htdvisser
htdvisser approved these changes Nov 26, 2019
View reviewed changes
Copy link
Contributor

@htdvisser htdvisser left a comment

Choose a reason for hiding this comment

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

LGTM. Can we add a unit test so that this would get caught if we break it again in the future?

@coveralls
Copy link

coveralls commented Nov 26, 2019 •
edited
Loading

Coverage Status

Coverage increased (+0.02%) to 73.824% when pulling d10849e on rvolosatovs:fix/js-app-key into 2763ae9 on TheThingsNetwork:master.

@rvolosatovs rvolosatovs added c/join server This is related to the Join Server security This is important for security blocking release This is blocking a release labels Nov 26, 2019
@rvolosatovs rvolosatovs merged commit bbadbf0 into TheThingsNetwork:master Nov 26, 2019
@rvolosatovs rvolosatovs deleted the fix/js-app-key branch November 26, 2019 11:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@htdvisser htdvisser htdvisser approved these changes

@johanstokking johanstokking Awaiting requested review from johanstokking

Assignees

@rvolosatovs rvolosatovs

Labels
blocking release This is blocking a release bug Something isn't working c/join server This is related to the Join Server security This is important for security
Projects
None yet
Milestone
November 2019
Development

Successfully merging this pull request may close these issues.
3 participants
@rvolosatovs @coveralls @htdvisser