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

Refactor cookie package to no longer depend on Echo #3457

Merged
merged 2 commits into from
Nov 13, 2020
Merged

Refactor cookie package to no longer depend on Echo #3457

merged 2 commits into from
Nov 13, 2020

Conversation

adriansmares
Copy link
Contributor

Summary

References #2627
Required for https://github.com/TheThingsIndustries/lorawan-stack/pull/2408 (in order to avoid migrating away from Echo later).

Changes

  • cookie.Cookie operations now work on http.ResponseWriter and *http.Request instead of echo.Context
  • Call sites and updates have been updated to follow on this change.

Testing

Unit testing.

Regressions

This simplifies the secure cookie management mechanism and removes one middleware. No regressions are expected.

Checklist

  • Scope: The referenced issue is addressed, there are no unrelated changes.
  • Compatibility: The changes are backwards compatible with existing API, storage, configuration and CLI, according to the compatibility commitments in README.md for the chosen target branch.
  • 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.

@adriansmares adriansmares added c/shared This is shared between components quickfix labels Nov 10, 2020
@adriansmares adriansmares added this to the November 2020 milestone Nov 10, 2020
@adriansmares adriansmares self-assigned this Nov 10, 2020
@github-actions github-actions bot added c/identity server This is related to the Identity Server ui/web This is related to a web interface labels Nov 10, 2020
@adriansmares adriansmares force-pushed the feature/2627-cookie-refactor branch from ade508e to cd1747a Compare November 10, 2020 16:55
kschiffer
kschiffer approved these changes Nov 13, 2020
View reviewed changes
Copy link
Contributor

@kschiffer kschiffer left a comment

Choose a reason for hiding this comment

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

Tested and LGTM

@adriansmares adriansmares merged commit 18909e4 into v3.10 Nov 13, 2020
@adriansmares adriansmares deleted the feature/2627-cookie-refactor branch November 13, 2020 09:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kschiffer kschiffer kschiffer approved these changes

@htdvisser htdvisser htdvisser approved these changes

@bafonins bafonins Awaiting requested review from bafonins

@pgalic96 pgalic96 Awaiting requested review from pgalic96

Assignees

@adriansmares adriansmares

Labels
c/identity server This is related to the Identity Server c/shared This is shared between components ui/web This is related to a web interface
Projects
None yet
Milestone
November 2020
Development

Successfully merging this pull request may close these issues.
3 participants
@adriansmares @htdvisser @kschiffer