diff options
Diffstat (limited to 'docs/dev')
| -rw-r--r-- | docs/dev/git.md | 148 | ||||
| -rw-r--r-- | docs/dev/git/branches.jpg | bin | 0 -> 72228 bytes | |||
| -rw-r--r-- | docs/dev/git/clean.png | bin | 0 -> 110840 bytes | |||
| -rw-r--r-- | docs/dev/git/squash.png | bin | 0 -> 29667 bytes | |||
| -rw-r--r-- | docs/dev/oidc.md | 175 |
5 files changed, 323 insertions, 0 deletions
diff --git a/docs/dev/git.md b/docs/dev/git.md new file mode 100644 index 0000000000..b747ff20c9 --- /dev/null +++ b/docs/dev/git.md @@ -0,0 +1,148 @@ +Some notes on how we use git +============================ + +On keeping the commit history clean +----------------------------------- + +In an ideal world, our git commit history would be a linear progression of +commits each of which contains a single change building on what came +before. Here, by way of an arbitrary example, is the top of `git log --graph +b2dba0607`: + +<img src="git/clean.png" alt="clean git graph" width="500px"> + +Note how the commit comment explains clearly what is changing and why. Also +note the *absence* of merge commits, as well as the absence of commits called +things like (to pick a few culprits): +[“pep8”](https://github.com/matrix-org/synapse/commit/84691da6c), [“fix broken +test”](https://github.com/matrix-org/synapse/commit/474810d9d), +[“oops”](https://github.com/matrix-org/synapse/commit/c9d72e457), +[“typo”](https://github.com/matrix-org/synapse/commit/836358823), or [“Who's +the president?”](https://github.com/matrix-org/synapse/commit/707374d5d). + +There are a number of reasons why keeping a clean commit history is a good +thing: + + * From time to time, after a change lands, it turns out to be necessary to + revert it, or to backport it to a release branch. Those operations are + *much* easier when the change is contained in a single commit. + + * Similarly, it's much easier to answer questions like “is the fix for + `/publicRooms` on the release branch?” if that change consists of a single + commit. + + * Likewise: “what has changed on this branch in the last week?” is much + clearer without merges and “pep8” commits everywhere. + + * Sometimes we need to figure out where a bug got introduced, or some + behaviour changed. One way of doing that is with `git bisect`: pick an + arbitrary commit between the known good point and the known bad point, and + see how the code behaves. However, that strategy fails if the commit you + chose is the middle of someone's epic branch in which they broke the world + before putting it back together again. + +One counterargument is that it is sometimes useful to see how a PR evolved as +it went through review cycles. This is true, but that information is always +available via the GitHub UI (or via the little-known [refs/pull +namespace](https://help.github.com/en/github/collaborating-with-issues-and-pull-requests/checking-out-pull-requests-locally)). + + +Of course, in reality, things are more complicated than that. We have release +branches as well as `develop` and `master`, and we deliberately merge changes +between them. Bugs often slip through and have to be fixed later. That's all +fine: this not a cast-iron rule which must be obeyed, but an ideal to aim +towards. + +Merges, squashes, rebases: wtf? +------------------------------- + +Ok, so that's what we'd like to achieve. How do we achieve it? + +The TL;DR is: when you come to merge a pull request, you *probably* want to +“squash and merge”: + +. + +(This applies whether you are merging your own PR, or that of another +contributor.) + +“Squash and merge”<sup id="a1">[1](#f1)</sup> takes all of the changes in the +PR, and bundles them into a single commit. GitHub gives you the opportunity to +edit the commit message before you confirm, and normally you should do so, +because the default will be useless (again: `* woops typo` is not a useful +thing to keep in the historical record). + +The main problem with this approach comes when you have a series of pull +requests which build on top of one another: as soon as you squash-merge the +first PR, you'll end up with a stack of conflicts to resolve in all of the +others. In general, it's best to avoid this situation in the first place by +trying not to have multiple related PRs in flight at the same time. Still, +sometimes that's not possible and doing a regular merge is the lesser evil. + +Another occasion in which a regular merge makes more sense is a PR where you've +deliberately created a series of commits each of which makes sense in its own +right. For example: [a PR which gradually propagates a refactoring operation +through the codebase](https://github.com/matrix-org/synapse/pull/6837), or [a +PR which is the culmination of several other +PRs](https://github.com/matrix-org/synapse/pull/5987). In this case the ability +to figure out when a particular change/bug was introduced could be very useful. + +Ultimately: **this is not a hard-and-fast-rule**. If in doubt, ask yourself “do +each of the commits I am about to merge make sense in their own right”, but +remember that we're just doing our best to balance “keeping the commit history +clean” with other factors. + +Git branching model +------------------- + +A [lot](https://nvie.com/posts/a-successful-git-branching-model/) +[of](http://scottchacon.com/2011/08/31/github-flow.html) +[words](https://www.endoflineblog.com/gitflow-considered-harmful) have been +written in the past about git branching models (no really, [a +lot](https://martinfowler.com/articles/branching-patterns.html)). I tend to +think the whole thing is overblown. Fundamentally, it's not that +complicated. Here's how we do it. + +Let's start with a picture: + + + +It looks complicated, but it's really not. There's one basic rule: *anyone* is +free to merge from *any* more-stable branch to *any* less-stable branch at +*any* time<sup id="a2">[2](#f2)</sup>. (The principle behind this is that if a +change is good enough for the more-stable branch, then it's also good enough go +put in a less-stable branch.) + +Meanwhile, merging (or squashing, as per the above) from a less-stable to a +more-stable branch is a deliberate action in which you want to publish a change +or a set of changes to (some subset of) the world: for example, this happens +when a PR is landed, or as part of our release process. + +So, what counts as a more- or less-stable branch? A little reflection will show +that our active branches are ordered thus, from more-stable to less-stable: + + * `master` (tracks our last release). + * `release-vX.Y.Z` (the branch where we prepare the next release)<sup + id="a3">[3](#f3)</sup>. + * PR branches which are targeting the release. + * `develop` (our "mainline" branch containing our bleeding-edge). + * regular PR branches. + +The corollary is: if you have a bugfix that needs to land in both +`release-vX.Y.Z` *and* `develop`, then you should base your PR on +`release-vX.Y.Z`, get it merged there, and then merge from `release-vX.Y.Z` to +`develop`. (If a fix lands in `develop` and we later need it in a +release-branch, we can of course cherry-pick it, but landing it in the release +branch first helps reduce the chance of annoying conflicts.) + +--- + +<b id="f1">[1]</b>: “Squash and merge” is GitHub's term for this +operation. Given that there is no merge involved, I'm not convinced it's the +most intuitive name. [^](#a1) + +<b id="f2">[2]</b>: Well, anyone with commit access.[^](#a2) + +<b id="f3">[3]</b>: Very, very occasionally (I think this has happened once in +the history of Synapse), we've had two releases in flight at once. Obviously, +`release-v1.2.3` is more-stable than `release-v1.3.0`. [^](#a3) diff --git a/docs/dev/git/branches.jpg b/docs/dev/git/branches.jpg new file mode 100644 index 0000000000..715ecc8cd0 --- /dev/null +++ b/docs/dev/git/branches.jpg Binary files differdiff --git a/docs/dev/git/clean.png b/docs/dev/git/clean.png new file mode 100644 index 0000000000..3accd7ccef --- /dev/null +++ b/docs/dev/git/clean.png Binary files differdiff --git a/docs/dev/git/squash.png b/docs/dev/git/squash.png new file mode 100644 index 0000000000..234caca3e4 --- /dev/null +++ b/docs/dev/git/squash.png Binary files differdiff --git a/docs/dev/oidc.md b/docs/dev/oidc.md new file mode 100644 index 0000000000..a90c5d2441 --- /dev/null +++ b/docs/dev/oidc.md @@ -0,0 +1,175 @@ +# How to test OpenID Connect + +Any OpenID Connect Provider (OP) should work with Synapse, as long as it supports the authorization code flow. +There are a few options for that: + + - start a local OP. Synapse has been tested with [Hydra][hydra] and [Dex][dex-idp]. + Note that for an OP to work, it should be served under a secure (HTTPS) origin. + A certificate signed with a self-signed, locally trusted CA should work. In that case, start Synapse with a `SSL_CERT_FILE` environment variable set to the path of the CA. + - use a publicly available OP. Synapse has been tested with [Google][google-idp]. + - setup a SaaS OP, like [Auth0][auth0] and [Okta][okta]. Auth0 has a free tier which has been tested with Synapse. + +[google-idp]: https://developers.google.com/identity/protocols/OpenIDConnect#authenticatingtheuser +[auth0]: https://auth0.com/ +[okta]: https://www.okta.com/ +[dex-idp]: https://github.com/dexidp/dex +[hydra]: https://www.ory.sh/docs/hydra/ + + +## Sample configs + +Here are a few configs for providers that should work with Synapse. + +### [Dex][dex-idp] + +[Dex][dex-idp] is a simple, open-source, certified OpenID Connect Provider. +Although it is designed to help building a full-blown provider, with some external database, it can be configured with static passwords in a config file. + +Follow the [Getting Started guide](https://github.com/dexidp/dex/blob/master/Documentation/getting-started.md) to install Dex. + +Edit `examples/config-dev.yaml` config file from the Dex repo to add a client: + +```yaml +staticClients: +- id: synapse + secret: secret + redirectURIs: + - '[synapse base url]/_synapse/oidc/callback' + name: 'Synapse' +``` + +Run with `dex serve examples/config-dex.yaml` + +Synapse config: + +```yaml +oidc_config: + enabled: true + skip_verification: true # This is needed as Dex is served on an insecure endpoint + issuer: "http://127.0.0.1:5556/dex" + discover: true + client_id: "synapse" + client_secret: "secret" + scopes: + - openid + - profile + user_mapping_provider: + config: + localpart_template: '{{ user.name }}' + display_name_template: '{{ user.name|capitalize }}' +``` + +### [Auth0][auth0] + +1. Create a regular web application for Synapse +2. Set the Allowed Callback URLs to `[synapse base url]/_synapse/oidc/callback` +3. Add a rule to add the `preferred_username` claim. + <details> + <summary>Code sample</summary> + + ```js + function addPersistenceAttribute(user, context, callback) { + user.user_metadata = user.user_metadata || {}; + user.user_metadata.preferred_username = user.user_metadata.preferred_username || user.user_id; + context.idToken.preferred_username = user.user_metadata.preferred_username; + + auth0.users.updateUserMetadata(user.user_id, user.user_metadata) + .then(function(){ + callback(null, user, context); + }) + .catch(function(err){ + callback(err); + }); + } + ``` + + </details> + + +```yaml +oidc_config: + enabled: true + issuer: "https://your-tier.eu.auth0.com/" # TO BE FILLED + discover: true + client_id: "your-client-id" # TO BE FILLED + client_secret: "your-client-secret" # TO BE FILLED + scopes: + - openid + - profile + user_mapping_provider: + config: + localpart_template: '{{ user.preferred_username }}' + display_name_template: '{{ user.name }}' +``` + +### GitHub + +GitHub is a bit special as it is not an OpenID Connect compliant provider, but just a regular OAuth2 provider. +The `/user` API endpoint can be used to retrieve informations from the user. +As the OIDC login mechanism needs an attribute to uniquely identify users and that endpoint does not return a `sub` property, an alternative `subject_claim` has to be set. + +1. Create a new OAuth application: https://github.com/settings/applications/new +2. Set the callback URL to `[synapse base url]/_synapse/oidc/callback` + +```yaml +oidc_config: + enabled: true + issuer: "https://github.com/" + discover: false + client_id: "your-client-id" # TO BE FILLED + client_secret: "your-client-secret" # TO BE FILLED + authorization_endpoint: "https://github.com/login/oauth/authorize" + token_endpoint: "https://github.com/login/oauth/access_token" + userinfo_endpoint: "https://api.github.com/user" + scopes: + - read:user + user_mapping_provider: + config: + subject_claim: 'id' + localpart_template: '{{ user.login }}' + display_name_template: '{{ user.name }}' +``` + +### Google + +1. Setup a project in the Google API Console +2. Obtain the OAuth 2.0 credentials (see <https://developers.google.com/identity/protocols/oauth2/openid-connect>) +3. Add this Authorized redirect URI: `[synapse base url]/_synapse/oidc/callback` + +```yaml +oidc_config: + enabled: true + issuer: "https://accounts.google.com/" + discover: true + client_id: "your-client-id" # TO BE FILLED + client_secret: "your-client-secret" # TO BE FILLED + scopes: + - openid + - profile + user_mapping_provider: + config: + localpart_template: '{{ user.given_name|lower }}' + display_name_template: '{{ user.name }}' +``` + +### Twitch + +1. Setup a developer account on [Twitch](https://dev.twitch.tv/) +2. Obtain the OAuth 2.0 credentials by [creating an app](https://dev.twitch.tv/console/apps/) +3. Add this OAuth Redirect URL: `[synapse base url]/_synapse/oidc/callback` + +```yaml +oidc_config: + enabled: true + issuer: "https://id.twitch.tv/oauth2/" + discover: true + client_id: "your-client-id" # TO BE FILLED + client_secret: "your-client-secret" # TO BE FILLED + client_auth_method: "client_secret_post" + scopes: + - openid + user_mapping_provider: + config: + localpart_template: '{{ user.preferred_username }}' + display_name_template: '{{ user.name }}' +``` |
