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

Application service can join remote federated room without profile set #399

Merged

Conversation

MadLittleMods
Copy link
Contributor

@MadLittleMods MadLittleMods commented Jun 29, 2022

Application service can join remote federated room without profile set

Synapse changes: matrix-org/synapse#13131 to address matrix-org/synapse#4778

remoteCharlie := deployment.Client(t, "hs2", remoteUserID)

t.Run("join remote federated room as application service user", func(t *testing.T) {
//t.Parallel()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see that we have t.Parallel() commented out elsewhere in this file. Do we care about it also being here?

})

// Join the AS to the remote federated room (without a profile set)
as.JoinRoom(t, roomID, []string{"hs2"})
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Test failing in Dendrite. I'm guessing we don't configure application service registrations in it like we do for Synapse.

https://github.com/matrix-org/complement/runs/7116659758?check_suite_focus=true#step:8:415

❌ TestJoinFederatedRoomFromApplicationServiceUser/join_remote_federated_room_as_application_service_user (180ms)
      federation_room_join_test.go:547: CSAPI.MustDoFunc response return non-2xx code: 401 Unauthorized - body: {"errcode":"M_UNKNOWN_TOKEN","error":"Unknown token"}
  2022/06/29 16:59:07 ============================================

Copy link
Contributor Author

Choose a reason for hiding this comment

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

cc @kegsay

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought AS support wasn't fully implemented yet in Complement?

Complement does support configuring application services. For Synapse, that means setting up the registration. But the application service server doesn't work yet. In the tests so far, I've only needed the as_token to do certain actions.

Application service servers not needed/supported until #221 (review)

Copy link
Member

Choose a reason for hiding this comment

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

Right, okay. Then yeah the Dendrite image won't read the registration files yet. Skip them on Dendrite please.

MadLittleMods added a commit to matrix-org/synapse that referenced this pull request Jul 5, 2022
@MadLittleMods MadLittleMods requested a review from kegsay July 6, 2022 16:44
@kegsay
Copy link
Member

kegsay commented Jul 11, 2022

I thought AS support wasn't fully implemented yet in Complement?

Copy link
Member

@kegsay kegsay left a comment

Choose a reason for hiding this comment

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

Otherwise LGTM

@MadLittleMods MadLittleMods merged commit ab94cac into main Jul 12, 2022
@MadLittleMods MadLittleMods deleted the madlittlemods/no-profile-necessary-to-remote-join-as-as branch July 12, 2022 18:52
@MadLittleMods
Copy link
Contributor Author

Thanks for the review @kegsay 🐊

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.

None yet

2 participants