We were sending `user.id` to the service, but if an admin token is used,
there is no `user.id.` Instead, there is
`user.internalAdminTokenUserId`. so we need to use the special method
`extractUserIdFromUser`.
This PR adds this implementation, and now the service correctly
retrieves the appropriate ID for admins.
Related to: https://github.com/Unleash/unleash/pull/5924
## About the changes
Schedules a best-effort task setting the value of
events.created_by_user_id based on what is found in the created_by
column and if it's capable of resolving that to a userid/a system id.
The process is executed in the events-store, it takes a chunk of events
that haven't been processed yet, attempts to join users and api_tokens
tables on created_by = username/email, loops through and tries to figure
out an id to set. Then updates the record.
---------
Co-authored-by: Gastón Fournier <gaston@getunleash.io>
## About the changes
Change the sorting of features to migrate created_by_user_id for, and
filter out unresolvable feature/users
Query tested manually in enterprise
## About the changes
Sets data migration of features and events created_by_user_id to
disabled by default
Map to promise and await all in created by user id migration for features
This escape with `??` double escaped the LIKE query causing no results.
This updates to using whereLike, which does the correct escaping for
string query.
## About the changes
Adds a scheduled task that every 5 seconds updates 500 entries in the
features table setting `created_by_user_id`.
It does this by looking at the related event, checks created_by and
joins users table for match on username or email, and joins api_tokens
table on username matches. Then picks either a users id if set, or uses
-42 (admin token user)
## About the changes
This PR replaces the old systemUser -1 in user-service.ts with the new
SYSTEM_USER -1337 and adds a migration to move events created_by = -1 to
-1337
## Discussion points
Does it make sense to do both of these things? Or should we skip the
migration? How would this behave in a large system with hundreds of
thousands of events, should this be split up?
Only triggers if there is any rows in client instances that have
sdk_version: unleash-edge with version < 17.0.0
The function that checks this memoizes the check for 10 minutes to avoid
scanning the client instances table too often.
Previously we used a killswitch and returned 404 if the feature was
enabled. This flips that to a default disabled toggle, that has to be
turned on to handle old Edge (pre 17.0.0) posting bulk metrics
We should use the enhanced flagResolver
Tested locally:
```
9:44:13 AM - Starting compilation in watch mode...
[dev:backend]
[dev:backend]
[dev:backend] 9:44:26 AM - Found 0 errors. Watching for file changes.
[dev:backend] [2024-01-23T09:44:27.498] [INFO] server-impl.js - DB migration: start
[dev:backend] [2024-01-23T09:44:27.499] [INFO] server-impl.js - Running migration with lock
[dev:backend] [2024-01-23T09:44:29.884] [INFO] server-impl.js - DB migration: end
```
This PR will allow us to use a feature flag with variants to control
whether or not we should show the comments field of the feedback form.
This will allow us to see whether we can increase feedback collection if
we reduce the load on the customer.
## About the changes
This was spotted while testing automated actions. Steps to reproduce:
1. Add an editor user
2. Get a PAT for the editor user
3. As Admin create a feature in a project where the editor user is not a
member and enable the feature
4. Try using the editor's PAT to modify the feature
5. As the editor create a project (you'd be made owner) and try the same
request but just change the project name for the new project just
created (don't change anything else)
**Expected behavior**: you can't disable the feature
**Actual behavior**: the feature is disabled
This does not happen when trying to turn on a flag because during the
turn-on process we do validate if the feature belongs to project when we
call updateStrategy:
c18a7c0dc2/src/lib/features/feature-toggle/feature-toggle-service.ts (L1751-L1764)
https://linear.app/unleash/issue/2-1856/add-typesafe-wrappers-over-prom-clients-metrics
As discussed on the latest knowledge sharing session, this adds typesafe
wrappers over prom client's metrics, requiring us to specify all the
configured labels for each metric.
This uses a functional approach and only exposes the methods that are
currently relevant to us, while also exposing the underlying instance of
the metric for an easy access if needed.
Since we often chain `labels` with `inc` in counters, this adds a
convenience `increment` method for counters which does both in a single
call.
Uses a new `URL_SAFE_BASIC` regex constant that checks for characters
that are commonly used in URL path sections: alphanumeric lowercase
characters, dashes and underscores.
This will allow us to re-use this constant in our server-side
validation.
So, this was causing a lot of ERROR in our logs, due to the metric
having gotten an extra label the last month.
Two things for this fix.
1. add the missing label to the two calls that did not have it added
2. update the log line to include the error as another argument to the
logger, so we actually get a stacktrace from the error.
### What
Adds Read and Write permissions for project administration settings
(user access, change request settings, default strategy, other).
### Why
On request from two large customers that wanted our RBAC controls to be
more granulated to easier be able to limit the access they granted their
users.
## About the changes
Whenever we get a call from an admin token we want to associate it with
the [admin token
user](4d42093a07/src/lib/types/core.ts (L34-L41)).
This should give us the needed audit for this type of calls that
currently were lacking a user id (we only stored a string with the token
name in the event log).
We consciously decided not to use `id` as the property to prevent any
unforeseen side effects. The reason is that only `IUser` type has an id
and adding an id to `IApiUser` might lead to confusion.
Since we've now added PAT's we really do recommend switching to those,
or for enterprises, we recommend using service accounts.
Admin tokens have an obvious disadvantage in that they're not connected
to any user, so actions performed by them are harder to audit.
This PR adds a killswitch for turning it off, in preparation for
deprecating them and ultimately removing them in the future.
## About the changes
This admin token user will help us differentiate actions performed by
the system from actions performed with an admin token.
Events created with an admin token should have the id of this user as
createdByUserId property and the username of the token used as the
createdBy property. i.e.
```json
{
"id": 11,
"type": "pat-created",
"createdBy": "admin-token",
"createdAt": "2024-01-16T13:16:27.887Z",
"createdByUserId": -42,
"data": {
"description": "admin-pat",
"expiresAt": "2024-02-15T13:16:25.586Z",
"secret": "***",
"userId": 1
},
"preData": null,
"tags": [],
"featureName": null,
"project": null,
"environment": null
}
```
## About the changes
EventsService is a dependency in most of our services. This creates
helper methods to create them easily and replace a few places where
we're creating them manually
This change removes the system user's email from the definition, instead
setting it to `null`. It also changes the name to "Unleash System".
The IUser interface doesn't allow `null` email addresses, so we change
the type definition of the system user to get around it. However, using
`null` (instead of just removing the property entirely) is useful
because when you get the system user from the DB, it's email value will
be null (after it has been nulled out).
As of today, there is nowhere in the Unleash system (OSS or Enterprise)
where we use the system user as an IUser (we only use username and ID).
So this change shouldn't break anything.
This should follow https://github.com/Unleash/unleash/pull/5849.
Lots of work here, mostly because I didn't want to turn off the
`noImplicitAnyLet` lint. This PR tries its best to type all the untyped
lets biome complained about (Don't ask me how many hours that took or
how many lints that was >200...), which in the future will force test
authors to actually type their global variables setup in `beforeAll`.
---------
Co-authored-by: Gastón Fournier <gaston@getunleash.io>
This change adjusts the exported `SYSTEM_USER` constant in `core.ts` to
match the one created in the migration in
`src/migrations/20231222071533-unleash-system-user.js`
The slight discrepancy between these two caused me some minor headache
when trying to write a test in enterprise.
It also removes the email because we have no inbox at that address (and
we probably don't want one).
For reference, the migration looks like this:
``` sql
ALTER TABLE users ADD COLUMN IF NOT EXISTS is_system BOOLEAN NOT NULL DEFAULT FALSE;
INSERT INTO users
(id, name, username, email, created_by_user_id, is_system)
VALUES
(-1337, 'Unleash System', 'unleash_system_user', 'system@getunleash.io', -1337, true);
```
This adds a bulk endpoint under `/api/client/metrics`. Accessible under
`/api/client/metrics/bulk`.
This allows us to piggyback on the need for an API user with access.
This PR mostly copies the behaviour from our `/edge/metrics` endpoint,
but it filters metrics to only include the environment that the token
has access to.
So a client token that has access to the `production` will not be
allowed to report metrics for the `development` environment. More
importantly, a `development` token will not be allowed to post metrics
for the `production` environment.
This PR adds the schedule suspended event to the slack-app and webhook
definitions.
It also slightly tweaks the markdown formatting of change requests to
add a definite article. This means the snapshot also needs to be
updated.
This metric was used while developing the optimal304 feature. The
feature flag has been removed and this data is not longer being
collected and this will remove the metric from Prometheus.
## About the changes
This allows us to encrypt emails at signup for demo users to further
secure our demo instance. Currently, emails are anonymized before
displaying events performed by demo users. But this means that emails
are stored at rest in our DB. By encrypting the emails at login, we're
adding another layer of protection.
This can be enabled with a flag and requires the encryption key and the
initialization vector (IV for short) to be present as environment
variables.
Related to our work for making Edge bulk metrics a 1st class citizen of
Unleash, this PR adds an X-Unleash-Version header to the response from
client registration.
Based on when we add the new `/api/client/metrics/bulk` endpoint, Edge
can use the response header from upstream to decide whether to post
metrics to `/edge/metrics` or `/api/client/metrics/bulk`.
If the kill switch is enabled unleash returns 404 and a json body explaining why a 404 was given, encouraging users to upgrade to the most recent version of Edge.
We've had a couple of misunderstandings from people surprised that
Unleash allows posts against the `/edge/validate` endpoint without an
API key. It is intentional that this endpoint does not require an
Authorization header, so this PR updates our OpenAPI spec to clarify
that there is no security required for `/edge/validate`
## About the changes
Migrations for:
- Adds column is_system to users
- Inserts unleash_system_user id -1337 to users
includes `is_system: false` in the activeUsers and activeAccounts where filter
Tested by running:
`
select * into users_pre_check from users where id > -1;
delete from users where id > -1;
`
before starting unleash, then inspecting users table after unleash has
started and verifying that an 'admin' user has been created.
---------
Co-authored-by: Christopher Kolstad <chriswk@getunleash.ai>
This backwards compatible change allows us to specify a schema `id`
(full path) which to me feels a bit better than specifying the schema
name as a string, since a literal string is prone to typos.
### Before
```ts
requestBody: createRequestSchema(
'createResourceSchema',
),
responses: {
...getStandardResponses(400, 401, 403, 415),
201: resourceCreatedResponseSchema(
'resourceSchema',
),
},
```
### After
```ts
requestBody: createRequestSchema(
createResourceSchema.$id,
),
responses: {
...getStandardResponses(400, 401, 403, 415),
201: resourceCreatedResponseSchema(
resourceSchema.$id,
),
},
```
With the recent changes it's common that we'll need both the id and
processed username from the auth user in the request, so this PR
provides some helper methods to simplify this.
## About the changes
Adds the new nullable column created_by_user_id to the data used by
feature-tag-store and feature-tag-service. Also updates openapi schemas.
