Mattermost Installation Flow #4534
Conversation
Mattermost install flow remove utils Fix jwt error Add install flow details Fix formatting Fix auth token
|
@matiasb Can you check this PR for the backend changes ! if the flow looks good I will make the front end changes to show the installation link to the user. |
|
this seems to be the start of #96 |
| permission_classes = (IsAuthenticated, RBACPermission) | ||
|
|
||
| rbac_permissions = { | ||
| "get": [RBACPermission.Permissions.INTEGRATIONS_WRITE], |
There was a problem hiding this comment.
It would be nice to move this logic to the backend itself, maybe using the existing GetChannelVerificationCode? (although it could be confusing because in this case the code would be for a mattermost instance instead of a telegram or msteams channel), or defining a new optional messaging backend step for this. In an ideal world, it should be possible to write a messaging backend without changing any code in the other apps. Will give it some thought.
There was a problem hiding this comment.
I gave some thought. One way it could be is via polymorphic association we could have a single model to generate the tokens for each backend apps with a type field but that might be slightly complicated to unify that approach for the existing apps token implementation.
|
|
||
|
|
||
| class MattermostBindings(APIView): | ||
| authentication_classes = (MattermostWebhookAuthTokenAuthentication,) |
There was a problem hiding this comment.
I think bindings should authenticate the user behind the request instead (or too), right?
There was a problem hiding this comment.
This call is to return the bindings supported by our app hence it has the token authentication. Once we decide on the bindings like /alert we will create separate paths (or reuse old ones) in which case those paths will have authentication of its own. Might also include the same token
| "requested_locations": ["/in_post", "/post_menu", "/command"], | ||
| "on_install": self._build_on_install_callback(auth_token=auth_token), | ||
| "bindings": self._build_bindings_callback(auth_token=auth_token), | ||
| "http": {"root_url": live_settings.MATTERMOST_WEBHOOK_HOST}, |
There was a problem hiding this comment.
I was just reviewing my notes and draft code, and I think that my original plan was to use JWT for the app auth/link (see also this example, and that's what the authenticate function was supposed to do in the WIP I shared; also, that should allow us identify the user making requests to OnCall from Mattermost).
So, I think, we could generate a random token to store in a Mattermost Server model (similar to the one in the gist). This token will be asked to the user during install, and we can then link the Mattermost App with Server (and the respective org).
The flow I'm imagining will be something like:
- Admin user goes to a "setup mattermost" endpoint
- this endpoint will setup a server instance with a generated token
- returns a manifest URL (no auth, this is public info) with the
use_jwtflag enabled
- User install app using the manifest URL; it should be asked for the generated secret
- manifest includes an
installURL (this URL could include the org public ID, and it should require a valid and matching token/secret, server-side). - when install endpoint is hit, this will complete the (OnCall Mattermost) server setup
- manifest includes an
And that's it (?). From that moment on, app should be installed and be able to talk to OnCall, and OnCall should have the information for the mattermost app linked to a specific org.
Makes sense? Sorry I didn't try to summarize this before, but I couldn't find the time to understand my notes and test a few things until now. I think that the basics for this are in the shared gist.
Let me know if you think this won't work, or if you prefer me to work on a better PoC following the above.
There was a problem hiding this comment.
Hi ! Correct me if my understanding is right,
- We have two endpoints,
/manifest--> Returns the manifest json/install--> The on_install call back
- Do we have to authenticate the
/manifestendpoint ? If No then wouldn't that mean this endpoint can be hit by anyone of the internet users ? Like I can hit the server of any customers oncall instance and spam them right ? - When the user hits the
Setup MattermostWe generate the JWT signing secret and create the JWT token with org id in it. The JWT token will be shared in the UI along with the Mattermost json url. When the user installs the app the Mattermost app will send the jwt token in the request header. For authenticating when we receive the jwt token we get the org id from the jwt payload and using the org id we get the signing secret from our DB for that org id and then verify the signature before proceeding Is that right ?
If (3) is the expected behaviour. When we do the link-user like linking the oncall user to Mattermost user ? I believe we will generate a separate token which needs to be sent when the user types /link-user command from Mattermost ? using which we link the user.
There was a problem hiding this comment.
Hi ! Correct me if my understanding is right,
1. We have two endpoints, * `/manifest` --> Returns the manifest json * `/install` --> The on_install call back
Right.
2. Do we have to authenticate the `/manifest` endpoint ? If No then wouldn't that mean this endpoint can be hit by anyone of the internet users ? Like I can hit the server of any customers oncall instance and spam them right ?
Hm.. Not exactly, because /install will require a valid JWT token. Also we should generate the manifest.json using org id info. So, when an admin generates the JWT token, they also get a URL to the manifest with a param indicating the org ID, which is used in the rendered /install URL. Then, anyone will be able to get the base manifest.json, but to make things work you still need a JWT token and (the correspondent) an org ID... we could also require a fresh enough token before allowing installation. Makes sense?
3. When the user hits the `Setup Mattermost` We generate the JWT signing secret and create the JWT token with org id in it. The JWT token will be shared in the UI along with the Mattermost json url. When the user installs the app the Mattermost app will send the jwt token in the request header. For authenticating when we receive the jwt token we get the org id from the jwt payload and using the org id we get the signing secret from our DB for that org id and then verify the signature before proceeding Is that right ?
Yeah, something like that is what I was thinking.
If (3) is the expected behaviour. When we do the link-user like linking the oncall user to Mattermost user ? I believe we will generate a separate token which needs to be sent when the user types /link-user command from Mattermost ? using which we link the user.
That's correct, we will still need some link-user command (for which we could generate a token, similar to telegram or msteams flows) to bind an OnCall account with a Mattermost user.
How does it sound?
There was a problem hiding this comment.
Agreed ! yeah /install will require the JWT which will have the org id but for the /manifest endpoint we should be using a token which maps to the org like we have in the current state of the PR ? or should we just send the org_id as plain text in the manifest url like /manifest?org_id=<ORG_ID> ?
Hm.. Not exactly, because /install will require a valid JWT token. Also we should generate the manifest.json using org id info. So, when an admin generates the JWT token, they also get a URL to the manifest with a param indicating the org ID, which is used in the rendered /install URL. Then, anyone will be able to get the base manifest.json, but to make things work you still need a JWT token and (the correspondent) an org ID... we could also require a fresh enough token before allowing installation. Makes sense?
I think overall I got the context expect the above point mentioned I will start making the changes and will update the PR over this week.
There was a problem hiding this comment.
Agreed ! yeah
/installwill require the JWT which will have the org id but for the/manifestendpoint we should be using a token which maps to the org like we have in the current state of the PR ? or should we just send the org_id as plain text in the manifest url like/manifest?org_id=<ORG_ID>?
👍 I think using the org_id as a query param should be ok, doesn't it? But if we are including the org id as part of the secret maybe we can even omit it here.
There was a problem hiding this comment.
Yeah makes sense ! I think I will skip the org id in the manifest and use the org id in the secret ! will start making the changes in this week !
What this PR does
This PR covers the installation flow,
/mattermost/setuppath which gives an manifest json link with the auth_token/appscommand with the manifest url to install the app in the instanceWhich issue(s) this PR closes
Closes [issue link here]
Checklist
pr:no public docsPR label added if not required)release:). These labels dictate how your PR willshow up in the autogenerated release notes.