Skip to content

Comments

External commits: add additional checks#2852

Merged
smatting merged 4 commits intodevelopfrom
FS-919-followup-simplify
Nov 21, 2022
Merged

External commits: add additional checks#2852
smatting merged 4 commits intodevelopfrom
FS-919-followup-simplify

Conversation

@smatting
Copy link
Contributor

@smatting smatting commented Nov 18, 2022 •
edited
Loading

This PR affects external comits:

  • adds requirement of Z-Client
  • adds a check that Z-Client matches the client identity in the keypackage that is added
  • adds a check that Z-User matches the client identity in the keypackage that is added
  • removes a duplicated call to addKeyPackageRef

Checklist

  • Add a new entry in an appropriate subdirectory of changelog.d
  • Read and follow the PR guidelines

@smatting smatting temporarily deployed to cachix November 18, 2022 15:44 Inactive
@smatting smatting temporarily deployed to cachix November 18, 2022 15:44 Inactive
@zebot zebot added the ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist label Nov 18, 2022
@smatting smatting temporarily deployed to cachix November 21, 2022 10:00 Inactive
@smatting smatting temporarily deployed to cachix November 21, 2022 10:00 Inactive
@mdimjasevic
Copy link
Contributor

mdimjasevic commented Nov 21, 2022 •
edited
Loading

I approve the PR, but it'd be good for someone else that hasn't been involved in it to take a look too.

@smatting smatting changed the title External Commits: Add keypackage checks and simplify ref update External commits: add additional checks Nov 21, 2022
@smatting smatting marked this pull request as ready for review November 21, 2022 10:04
@mdimjasevic mdimjasevic self-requested a review November 21, 2022 10:05
pcapriotti
pcapriotti approved these changes Nov 21, 2022
View reviewed changes
Copy link
Contributor

@pcapriotti pcapriotti left a comment

Choose a reason for hiding this comment

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

LGTM

services/galley/src/Galley/API/MLS/Message.hs Outdated
Comment on lines 717 to 718
-- for_ senderClient $ \senderClient' ->
-- pure ()
Copy link
Contributor

@pcapriotti pcapriotti Nov 21, 2022

Choose a reason for hiding this comment

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

Leftover code?

Copy link
Contributor Author

@smatting smatting Nov 21, 2022

Choose a reason for hiding this comment

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

Ah forgot to push

@smatting smatting temporarily deployed to cachix November 21, 2022 10:51 Inactive
@smatting smatting temporarily deployed to cachix November 21, 2022 10:51 Inactive
@smatting smatting merged commit df49506 into develop Nov 21, 2022
@smatting smatting deleted the FS-919-followup-simplify branch November 21, 2022 10:52
This was referenced Nov 25, 2022
Closed
Closed
Closed
Merged
Merged
@zebot zebot mentioned this pull request Jan 12, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pcapriotti pcapriotti pcapriotti approved these changes

+1 more reviewer

@mdimjasevic mdimjasevic mdimjasevic approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@smatting @mdimjasevic @pcapriotti @zebot