Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

ModuleAPI SSO auth callbacks #15207

Merged

Conversation

yasinishyn
Copy link
Contributor

@yasinishyn yasinishyn commented Mar 4, 2023

This PR implements a callaback that is executed on any SSO auth (registration and logins). This introduces an ability to add custom callbacks using ModuleAPI.

Reasoning:

Pull Request Checklist

  • Pull request is based on the develop branch
  • Pull request includes a changelog file. The entry should:
    • Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from EventStore to EventWorkerStore.".
    • Use markdown where necessary, mostly for code blocks.
    • End with either a period (.) or an exclamation mark (!).
    • Start with a capital letter.
    • Feel free to credit yourself, by adding a sentence "Contributed by @github_username." or "Contributed by [Your Name]." to the end of the entry.
  • Pull request includes a sign off
  • Code style is correct
    (run the linters)

Signed-off-by: Andrii Yasynyshyn [email protected]

@clokep
Copy link
Member

clokep commented Mar 6, 2023

Looks like this is failing lint.

@yasinishyn
Copy link
Contributor Author

Linters are fixed. @clokep could you please take a look again?

@clokep
Copy link
Member

clokep commented Mar 10, 2023

I kicked off the build again, this is in our review queue. 👍

@yasinishyn
Copy link
Contributor Author

I kicked off the build again, this is in our review queue. 👍

Thank you! :)

@MatMaul
Copy link
Contributor

MatMaul commented Mar 20, 2023

@anoadragon453 what do you think of on_sso_login vs having a more generic on_login as proposed ?

On my side I think I would favor just going for a generic on_login, someone will probably need it someday and then we would have to keep both for not much gain.

@anoadragon453
Copy link
Member

A generic on_user_login sounds reasonable to me. I found an issue that relates to this and wrote up what I think would be a reasonable design at #10487 (comment).

@yasinishyn
Copy link
Contributor Author

@MatMaul @anoadragon453
Please let me know if you need my help here.
Will be happy to implement a more generic on_user_login callback.

@MatMaul
Copy link
Contributor

MatMaul commented Mar 23, 2023

@yasinishyn that would be nice, thank you 🙂

@clokep clokep removed the request for review from a team March 23, 2023 13:49
@clokep
Copy link
Member

clokep commented Mar 23, 2023

(Removing from the review queue since it sounds like changes are planned?)

@yasinishyn
Copy link
Contributor Author

I am sorry for the late update on this.
Please see a more generic implementation of the callback added in the last comment.

Copy link
Member

@clokep clokep left a comment

Choose a reason for hiding this comment

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

There's a few linting errors.

synapse/handlers/account_validity.py Outdated Show resolved Hide resolved
@clokep clokep requested a review from a team November 27, 2023 13:52
@github-actions github-actions bot deployed to PR Documentation Preview November 27, 2023 13:53 Active
Copy link
Member

@erikjohnston erikjohnston left a comment

Choose a reason for hiding this comment

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

Makes sense to me! Thank you

@erikjohnston erikjohnston enabled auto-merge (squash) December 1, 2023 14:11
@erikjohnston erikjohnston merged commit 63d96bf into matrix-org:develop Dec 1, 2023
40 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants