Simplified Upload Tokens for CI Usage #91

mitsuhiko · 2023-05-15T11:41:18Z

This RFC proposes adding org level tokens that contain meta information which simplifies user flows and lets clients forgo of necessary other information.

Rendered RFC

Refs the following issues:

text/0091-ci-upload-tokens.md

markstory · 2023-05-15T15:11:52Z

text/0091-ci-upload-tokens.md

+The proposed token format is undecided so far. The goals of the token align generally with
+both [Macaroons](http://macaroons.io/) and [Biscuit](https://www.biscuitsec.org). Unfortunately
+the former standard has never seen much attention, and the latter is pretty new and not
+particularly proven. Either system however permits adding additional restrictions to the
+token which make them a perfect choice for the use in our pipeline. Biscuits however seem
+quite promising. The idea of biscuits is that the token itself holds _facts_ which and
+can be further constraints via _checks_ and they are checked against a datalog inspired
+_policies_.
+
+One of the benefits of having the tokens carry this data is that the token alone has enough
+information available to route to a Sentry installation. This means that `sentry-cli` or
+any other tool _just_ needs the token to even determine the host that the token should be
+sent against. This benefit also applies to JWT or PASETO tokens which can be considered
+for this as well. The RFC here thus proposes two potential options: A **Biscuit** token
+format and a **JWT** token format.


Couldn't the scoping we need be achieved with Paseto or JWT? Those seems like safer bets than biscuit/macroons.

I’m not particularly sold on Biscuit but it’s interesting. Based on my experience with Macaroons with PyPI I don’t think that format has a future. What is nice about Biscuit is that a token can be restricted by the token holder, something that JWT/PASETO cannot do.

markstory · 2023-05-15T15:13:33Z

text/0091-ci-upload-tokens.md

+
+* `site`: references the target API URL that should be used. A token will always have a
+ site in it and clients are not supposed to provide a fallback. For instance this
+ would be `https://myorg.sentry.io/`.


For hybrid cloud we won't be able to have API endpoints use orgslug.sentry.io without proxying through control silo. However, we could have structured tokens contain regional domains eg. us.sentry.io

whys this different than org? why do we need this site component?

For hybrid cloud the benefit is that you can (without a preflight) hit the right instance. Particularly for debug file uploads we do not want them to go via the central proxy at all times. For single tenant they all require different URLs anyways and today the docs are quite blind on what you have to reconfigure to point it to the right single tenant instance.

text/0091-ci-upload-tokens.md

Co-authored-by: Mark Story <[email protected]>

mitsuhiko · 2023-05-16T18:23:18Z

For future readers: it's unlikely we will go down the path of using Biscuits. They are too complex, so don't be put off by the part of that in the RFC.

scefali · 2023-05-16T18:27:30Z

@mitsuhiko Might be a dumb question, but can we expose these tokens in the docs UI? It would be amazing if we could just automatically generate a token and put it in the docs link like we do for the user/org

dcramer · 2023-05-17T20:14:19Z

text/0091-ci-upload-tokens.md

+* `org`: a token is uniquely bound to an org, so the slug of that org is also always
+ contained. Note that the slug is used rather than an org ID as the clients typically
+ need these slugs to create API requests.
+* `projects`: a token can be valid for more than one project. For operations such as


we dont need project for the use case we have - so do we really need that as part of this spec?

Having the project as part of the token would make it possible that just the token is all that is needed to upload debug files or source maps and automatically associate it with the right project without further configuration needed. Otherwise it's yet again an extra piece of information that needs to be configured separately.

dcramer · 2023-05-17T20:15:22Z

A more broad question: why dont we just make an org level token that can be provisioned? why do we need an entire new token format/etc to solve such a simplistic problem?

OrgToken { token: string; scope: ["projects:symbols"] }

philipphofmann · 2023-05-19T11:57:50Z

A more broad question: why dont we just make an org level token that can be provisioned? why do we need an entire new token format/etc to solve such a simplistic problem?
OrgToken { token: string; scope: ["projects:symbols"] }

I had similar thoughts. If I'm not mistaken, the underlying problem is that users have a hard time understanding which token to create or use for uploading symbols. We could either solve this by what @dcramer suggested and/or improve the UI.

There are several ways how we could solve this. We could add a new some kind of symbols integration. Furthermore, if the user doesn't have the right permissions, we currently show the warning These settings can only be edited by users with the organization owner or manager role. We could add a request token button, that notifies the people with the proper permissions to create such a token or an integration.

mitsuhiko · 2023-05-22T09:51:29Z

A more broad question: why dont we just make an org level token that can be provisioned? why do we need an entire new token format/etc to solve such a simplistic problem?

To upload correctly the following pieces of information are needed:

target site: sentry.io vs foo.my.sentry.io (and whatever hybrid cloud needs)
org slug
project slug
auth token

By encoding most of this information into the token it means that most configuration is no longer necessary.

mitsuhiko · 2023-05-22T10:30:02Z

@scefali the goal is for sure that the docs can directly get such a token.

HazAT · 2023-05-22T12:52:27Z

@scefali think of an experience similar to this

evanpurkhiser · 2023-05-23T08:38:23Z

text/0091-ci-upload-tokens.md

+ devtool: "source-map",
+ plugins: [
+ new SentryWebpackPlugin({
+ authToken: "AUTO GENERATED TOKEN HERE",


Should this still be something like process.env.SNTRY_STRUCTURED_TOKEN for this example? This seems to imply that it is not a secret

@evanpurkhiser the idea is that the docs directly inject the token same as they do for the slugs.

mydea · 2023-06-02T11:52:06Z

text/0091-ci-upload-tokens.md

+
+We want to encode certain information into the tokens. The following attributes are defined:
+
+* `sentry_site`: references the target API URL that should be used. A token will always have a


Maybe specify here that this is options['system.url-prefix'], for reference?

referencing this comment from @markstory here: getsentry/sentry#50409 (comment)

In the future, organizations will have a couple URLs:

sentryUrl which is our root domain and is the same as system.url-prefix. ex. sentry.io
organizationUrl which is domain for the organization's UI. ex acme.sentry.io
regionUrl Which is the domain that the organization's API endpoints are available on. ex us.sentry.io
Any API requests sent to sentryUrl will be routed to the correct region, but will suffer a latency penalty (because we're proxying the request to the region) and the request will go to the US (this can matter for EU customers).

For this scenario, I think you'll want both the sentryUrl and regionUrl available in the JWT. There are a small number of API endpoints that are only available on sentryUrl (users, integrations, sentry apps).

So based on this, I would say we should add sentry_url and sentry_region_url fields instead of sentry_site?

So based on this, I would say we should add sentry_url and sentry_region_url fields instead of sentry_site?

Yes, having the sentry_url and sentry_region_url will work much better in the multi-region future.

why is anything other than the api url needed?

mydea · 2023-06-02T11:53:54Z

text/0091-ci-upload-tokens.md

+
+## Token Issuance
+
+The purpose of this change is to allow any organization member to issue tokens with little


I wonder if we should restrict revocation to managers/owners? 🤔 this has the downside that a user cannot revoke a token they just created, but may be a reasonable safety net (even to disallow accidental deletion)?

mdtro · 2023-06-05T14:49:48Z

text/0091-ci-upload-tokens.md

+```json
+{
+ "iss": "sentry.io",
+ "iat": 1684154626,
+ "sentry_site": "https://myorg.sentry.io/",
+ "sentry_org": "myorg"
+}


We should add a nonce field here so these are less predictable/guessable. A random string will suffice, the longer, the better. With this current format, someone would just need to brute force the iat to find a value token.

We should also store the tokens as hashed values in the database.

Then when it comes to verifying an inbound request, it's a matter of:

hash the provided token

check the database for a hash of equal value

if one is found, decode the provided token

perform requested operation (pending authorization)

We would like to at least show the last characters of the token in the UI after creation (e.g. "ends with ABCD"). How important is it that we hash the token in the DB? If it is a definite requirement, then we'll need to store the trailing characters separately as well.

Regards to the none, definitely! Very good point to add to the spec.

Are the last characters to help identify the token? If so, Could we instead expand the model to have a name or description or both? Then the user can provide a meaningful value to them.

yes, the idea is to identify the token. We will also have a name, but since part of this will be about auto-generating tokens from other parts of the UI (e.g. in docs), where we do not want to prompt the user to enter a name, I think there is still value in seeing the last characters of the token in order to identify which of the tokens in the overview is the one I actually ended up using, etc.

So what I would propose is:

store the token as hash

store the last 4 characters of the token in plain text in a separate field

Users can also provide a name (but since we expect to auto-generate the tokens in many cases from other places of the UI, we cannot expect to always have a great/unique name there)

Does that sound OK?

Yeah, that is okay. 👍

Co-authored-by: Mark Story <[email protected]>

mydea

I merged in the changes from #102, which should hopefully address/implement feedback brought up in comments here.

ref #50144 based on RFC getsentry/rfcs#91

CI Enabling Upload Tokens

8d49e5a

mitsuhiko force-pushed the feature/upload-tokens branch from 15e6269 to 8d49e5a Compare May 15, 2023 11:41

mitsuhiko changed the title ~~CI Enabling Upload Tokens~~ Simplified Upload Tokens for CI Usage May 15, 2023

mitsuhiko added 6 commits May 15, 2023 13:44

Added Hybrid Cloud / Single Tenant note

2f9c89f

Updated spec to mention JWT

1008238

Added discussion points

f55b237

Added JWT example

6d83429

Syntax highlighting

c7a7bcd

Chop off prefix

5639699

markstory reviewed May 15, 2023

View reviewed changes

Apply suggestions from code review

179195a

Co-authored-by: Mark Story <[email protected]>

smeubank mentioned this pull request May 16, 2023

Simplify Tokens for CI getsentry/team-webplatform-meta#67

Closed

dcramer reviewed May 17, 2023

View reviewed changes

No biscuit

5eff566

evanpurkhiser reviewed May 23, 2023

View reviewed changes

Update RFC on recent conversations

404b151

stephanie-anderson mentioned this pull request May 30, 2023

Draft: Org/Project Auth Tokens getsentry/sentry#49943

Closed

mydea mentioned this pull request Jun 1, 2023

Add new model OrgAuthToken based on existing ApiToken model getsentry/sentry#50144

Closed

mydea reviewed Jun 2, 2023

View reviewed changes

mdtro reviewed Jun 5, 2023

View reviewed changes

mydea mentioned this pull request Jun 7, 2023

feat: add OrgAuthToken model getsentry/sentry#50409

Merged

mydea added a commit that referenced this pull request Jun 12, 2023

adjustments to RFC #91

7524b7e

mydea added a commit that referenced this pull request Jun 12, 2023

adjustments to RFC #91

83282a2

mydea mentioned this pull request Jun 12, 2023

Adjustments to RFC #91 #102

Merged

Adjustments to RFC #91 (#102)

7b6cfe0

Co-authored-by: Mark Story <[email protected]>

mydea approved these changes Jun 13, 2023

View reviewed changes

mitsuhiko merged commit 54c6937 into main Jun 13, 2023
2 checks passed

mitsuhiko deleted the feature/upload-tokens branch June 13, 2023 15:20

mydea added a commit to getsentry/sentry that referenced this pull request Jun 14, 2023

feat: add OrgAuthToken model (#50409)

fdc8f5a

ref #50144 based on RFC getsentry/rfcs#91

mydea mentioned this pull request Jun 21, 2023

feat: Update RFC#91 with new token format #105

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Simplified Upload Tokens for CI Usage #91

Simplified Upload Tokens for CI Usage #91

mitsuhiko commented May 15, 2023 •

edited

markstory May 15, 2023

mitsuhiko May 15, 2023

markstory May 15, 2023

dcramer May 17, 2023

mitsuhiko May 22, 2023

mitsuhiko commented May 16, 2023

scefali commented May 16, 2023

dcramer May 17, 2023

mitsuhiko May 22, 2023

dcramer commented May 17, 2023

philipphofmann commented May 19, 2023 •

edited

mitsuhiko commented May 22, 2023

mitsuhiko commented May 22, 2023

HazAT commented May 22, 2023

evanpurkhiser May 23, 2023

mitsuhiko May 23, 2023

mydea Jun 2, 2023

mydea Jun 12, 2023

markstory Jun 12, 2023

dcramer Jun 12, 2023

mydea Jun 2, 2023

mdtro Jun 5, 2023 •

edited

mydea Jun 7, 2023

mdtro Jun 7, 2023

mydea Jun 9, 2023

mdtro Jun 9, 2023

mydea left a comment


		We want to encode certain information into the tokens. The following attributes are defined:

		* `sentry_site`: references the target API URL that should be used. A token will always have a


		## Token Issuance

		The purpose of this change is to allow any organization member to issue tokens with little

Simplified Upload Tokens for CI Usage #91

Simplified Upload Tokens for CI Usage #91

Conversation

mitsuhiko commented May 15, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mitsuhiko commented May 16, 2023

scefali commented May 16, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dcramer commented May 17, 2023

philipphofmann commented May 19, 2023 • edited

mitsuhiko commented May 22, 2023

mitsuhiko commented May 22, 2023

HazAT commented May 22, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mdtro Jun 5, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mydea left a comment

Choose a reason for hiding this comment

mitsuhiko commented May 15, 2023 •

edited

philipphofmann commented May 19, 2023 •

edited

mdtro Jun 5, 2023 •

edited