# Which Problems Are Solved
Potential nil pointers leading to a panic in the login UI.
# How the Problems Are Solved
As of now the login UI did not always check if the authRequest was
actually retrieved form the database, which is ok for some endpoints
which can also be called outside of an auth request.
There are now methods added to ensure the request is loaded.
# Additional Changes
None
# Additional Context
Closes https://github.com/zitadel/DevOps/issues/55
# Which Problems Are Solved
Curretly loading targets fails on cockraochdb because the order of
`with`-statements is wrong.
# How the Problems Are Solved
Changed the order of queries in the statements.
If the feature is enabled the new packages are used to query org by id
Part of: https://github.com/zitadel/zitadel/issues/7639
### Definition of Ready
- [x] I am happy with the code
- [x] Short description of the feature/issue is added in the pr
description
- [x] PR is linked to the corresponding user story
- [ ] Acceptance criteria are met
- [ ] All open todos and follow ups are defined in a new ticket and
justified
- [ ] Deviations from the acceptance criteria and design are agreed with
the PO and documented.
- [x] No debug or dead code
- [x] My code has no repetitions
- [ ] Critical parts are tested automatically
- [ ] Where possible E2E tests are implemented
- [ ] Documentation/examples are up-to-date
- [ ] All non-functional requirements are met
- [x] Functionality of the acceptance criteria is checked manually on
the dev system.
# Which Problems Are Solved
Currently on instance setup there is only a possibility to either use a
human or a machine user and not both at creation.
# How the Problems Are Solved
The logic in the instance setup is refactored and changed so there is
not an exclusion.
# Additional Changes
Refactoring, so that unit testing is possible to add for the different
elements of an instance setup.
# Additional Context
Closes#6430
# Which Problems Are Solved
There is confusing ambiguity in the error messages for setting too long
passwords in different places.
# How the Problems Are Solved
A check for maximum password length is added so it's clear that
passwords can't exceed a maximum length of 70 or 72 bytes.
Password validation now provides a live updating check mark or cross
mark to indicate if the maximum length requirement is met.
# Additional Changes
Clarified requirement descriptions on the registration page with
complete sentences.
# Additional Context
Closes#6301
---------
Co-authored-by: Elio Bischof <elio@zitadel.com>
# Which Problems Are Solved
After https://github.com/zitadel/zitadel/pull/7822 was merged we
discovered that
v2 tokens that where obtained through an IDP using the v1 login, can't
be used for
zitadel API calls.
- Because we used to store the AMR claim on the auth request, but
internally use the domain.UserAuthMethod type. AMR has no notion of an
IDP login, so that "factor" was lost
during conversion. Rendering those v2 tokens invalid on the zitadel API.
- A wrong check on machine user tokens falsly allowed some tokens to be
valid
- The client ID was set to tokens from client credentials and JWT
profile, which made client queries fail in the validation middleware.
The middleware expects client ID unset for machine users.
# How the Problems Are Solved
Store the domain.AuthMethods directly in the auth requests and session,
instead of using AMR claims with lossy conversion.
- IDPs have seperate auth method, which is not an AMR claim
- Machine users are treated specialy, eg auth methods are not required.
- Do not set the client ID for client credentials and JWT profile
# Additional Changes
Cleaned up mostly unused `oidc.getInfoFromRequest()`.
# Additional Context
- Bugs were introduced in https://github.com/zitadel/zitadel/pull/7822
and not yet part of a release.
- Reported internally.
# Which Problems Are Solved
ZITADEL currently always uses
`urn:oasis:names:tc:SAML:2.0:nameid-format:persistent` in SAML requests,
relying on the IdP to respect that flag and always return a peristent
nameid in order to be able to map the external user with an existing
user (idp link) in ZITADEL.
In case the IdP however returns a
`urn:oasis:names:tc:SAML:2.0:nameid-format:transient` (transient)
nameid, the attribute will differ between each request and it will not
be possible to match existing users.
# How the Problems Are Solved
This PR adds the following two options on SAML IdP:
- **nameIDFormat**: allows to set the nameid-format used in the SAML
Request
- **transientMappingAttributeName**: allows to set an attribute name,
which will be used instead of the nameid itself in case the returned
nameid-format is transient
# Additional Changes
To reduce impact on current installations, the `idp_templates6_saml`
table is altered with the two added columns by a setup job. New
installations will automatically get the table with the two columns
directly.
All idp unit tests are updated to use `expectEventstore` instead of the
deprecated `eventstoreExpect`.
# Additional Context
Closes#7483Closes#7743
---------
Co-authored-by: peintnermax <max@caos.ch>
Co-authored-by: Stefan Benz <46600784+stebenz@users.noreply.github.com>
Add a check for circular includes in action v2 executions, so that no
self-includes or infinite loops can happen.
Closes#7445
### Definition of Ready
- [x] I am happy with the code
- [x] Short description of the feature/issue is added in the pr
description
- [x] PR is linked to the corresponding user story
- [x] Acceptance criteria are met
- [x] All open todos and follow ups are defined in a new ticket and
justified
- [x] Deviations from the acceptance criteria and design are agreed with
the PO and documented.
- [x] No debug or dead code
- [x] My code has no repetitions
- [x] Critical parts are tested automatically
- [x] Where possible E2E tests are implemented
- [x] Documentation/examples are up-to-date
- [x] All non-functional requirements are met
- [x] Functionality of the acceptance criteria is checked manually on
the dev system.
---------
Co-authored-by: Livio Spring <livio.a@gmail.com>
# Which Problems Are Solved
During the implementation of #7486 it was noticed, that projections in
the `auth` database schema could be blocked.
Investigations suggested, that this is due to the use of
[GORM](https://gorm.io/index.html) and it's inability to use an existing
(sql) transaction.
With the improved / simplified handling (see below) there should also be
a minimal improvement in performance, resp. reduced database update
statements.
# How the Problems Are Solved
The handlers in `auth` are exchanged to proper (sql) statements and gorm
usage is removed for any writing part.
To further improve / simplify the handling of the users, a new
`auth.users3` table is created, where only attributes are handled, which
are not yet available from the `projections.users`,
`projections.login_name` and `projections.user_auth_methods` do not
provide. This reduces the events handled in that specific handler by a
lot.
# Additional Changes
None
# Additional Context
relates to #7486
# Which Problems Are Solved
- In #7929 it was detected that it would be better to show the
activate/deactivate action for a SMTP configuration in the wizard as for
some users it'd not be intuitive that the SMTP provider must be
activated so Zitadel can use it to send notifications.
# How the Problems Are Solved
- When a new SMTP provider is added or updated, the wizard has a new
step that allow us to activate or deactivate the provider configured in
the previous step. The following video shows the new wizard:
https://github.com/zitadel/zitadel/assets/30386061/178234d6-73dc-4719-af0b-1d6f19bf3f7d
# Additional Context
- Closes#7929
# Which Problems Are Solved
The session update requires the current session token as argument.
Since this adds extra complexity but no real additional security and
prevents case like magic links, we want to remove this requirement.
We still require the session token on other resouces / endpoints, e.g.
for finalizing the auth request or on idp intents.
# How the Problems Are Solved
- Removed the session token verifier in the Update Session GRPc call.
- Removed the session token from login UI examples session update calls
# Additional Changes
- none
# Additional Context
- Closes#7883
# Which Problems Are Solved
When poviding `select_account` in an OIDC auth request, ZITADEL would
always show the account selection page even if there aren't any user
sessions to choose and the user would then need to click the `Other
User` button to be presented the login page.
# How the Problems Are Solved
This PR changes the behavior and ignores the `select_account` prompt in
case there aren't any existing user sessions and will directly present
the login page.
# Additional Changes
None
# Additional Context
Closes#7213
* implement code exchange
* port tokenexchange to v2 tokens
* implement refresh token
* implement client credentials
* implement jwt profile
* implement device token
* cleanup unused code
* fix current unit tests
* add user agent unit test
* unit test domain package
* need refresh token as argument
* test commands create oidc session
* test commands device auth
* fix device auth build error
* implicit for oidc session API
* implement authorize callback handler for legacy implicit mode
* upgrade oidc module to working draft
* add missing auth methods and time
* handle all errors in defer
* do not fail auth request on error
the oauth2 Go client automagically retries on any error. If we fail the auth request on the first error, the next attempt will always fail with the Errors.AuthRequest.NoCode, because the auth request state is already set to failed.
The original error is then already lost and the oauth2 library does not return the original error.
Therefore we should not fail the auth request.
Might be worth discussing and perhaps send a bug report to Oauth2?
* fix code flow tests by explicitly setting code exchanged
* fix unit tests in command package
* return allowed scope from client credential client
* add device auth done reducer
* carry nonce thru session into ID token
* fix token exchange integration tests
* allow project role scope prefix in client credentials client
* gci formatting
* do not return refresh token in client credentials and jwt profile
* check org scope
* solve linting issue on authorize callback error
* end session based on v2 session ID
* use preferred language and user agent ID for v2 access tokens
* pin oidc v3.23.2
* add integration test for jwt profile and client credentials with org scopes
* refresh token v1 to v2
* add user token v2 audit event
* add activity trigger
* cleanup and set panics for unused methods
* use the encrypted code for v1 auth request get by code
* add missing event translation
* fix pipeline errors (hopefully)
* fix another test
* revert pointer usage of preferred language
* solve browser info panic in device auth
* remove duplicate entries in AMRToAuthMethodTypes to prevent future `mfa` claim
* revoke v1 refresh token to prevent reuse
* fix terminate oidc session
* always return a new refresh toke in refresh token grant
---------
Co-authored-by: Livio Spring <livio.a@gmail.com>
* fix: import totp in add human user with secret
* fix: import totp in add human user with secret
* fix: import totp in add human user with secret
* fix: review comment changes
* fix: correct resourceowner of intent to instance
* fix: correct resourceowner of intent to instance
* fix: correct resourceowner of intent to instance
* fix: correct resourceowner of intent to instance
* fix: correct resourceowner of intent to instance
* docs: expand the login example with org specific parameters
* fix: existence of idp is not checked through resourceowner
* fix: existence of idp is not checked through resourceowner
* fix: existence of idp is not checked through resourceowner
---------
Co-authored-by: Livio Spring <livio.a@gmail.com>
* fix: check password complexity policy on password change and respect require_change
* pass changeRequired where available and add tests
* fix requested changes
---------
Co-authored-by: Stefan Benz <46600784+stebenz@users.noreply.github.com>
* fix: poc outlook.com now works login auth
* fix: remove port arg from smtpAuth
* fix: add outlook provider and custom email placeholder
* fix: minor typo in contributing docs
* fix: use zerrors package
* fix: typo for idp and smtp providers
---------
Co-authored-by: Max Peintner <max@caos.ch>
* docs: translate missing event types
* fix: wrong example in api docs
* Update internal/static/i18n/cs.yaml
Co-authored-by: Livio Spring <livio.a@gmail.com>
---------
Co-authored-by: Livio Spring <livio.a@gmail.com>
* fix: improve secret generation for apple idp
* remove accidental commit
* change exp time
* change exp time
* change exp time
* change exp time
(cherry picked from commit 6ab06aa249)
* Allow using a local RSA key for machine keys
* Add check for key validity
* Fix naming error
* docs: provide translations of invalid key
---------
Co-authored-by: Livio Spring <livio.a@gmail.com>
We are trying to reproduce a few 500 responses we observe on zitadel cloud's token endpoint.
As in the past these were caused by wrongly encoded or encrypted refresh tokens, I created a integration test which tries to reproduce 500 errors by sending invalid refresh tokens.
The added test does not reproduce 500s, all returned errors are in the 400 range as they should. However, as the test is already written, we might as well include them.
Related to #7765
Co-authored-by: Livio Spring <livio.a@gmail.com>
* fix(oidc): roles in userinfo for client credentials token
When tokens were obtained using the client credentials grant,
with audience and role scopes, userinfo would not return the role claims. This had multiple causes:
1. There is no auth request flow, so for legacy userinfo project data was never attached to the token
2. For optimized userinfo, there is no client ID that maps to an application. The client ID for client credentials is the machine user's name. There we can't obtain a project ID. When the project ID remained empty, we always ignored the roleAudience.
This PR fixes situation 2, by always taking the roleAudience into account, even when the projectID is empty. The code responsible for the bug is also refactored to be more readable and understandable, including additional godoc.
The fix only applies to the optimized userinfo code introduced in #7706 and released in v2.50 (currently in RC). Therefore it can't be back-ported to earlier versions.
Fixes#6662
* chore(deps): update all go deps (#7764)
This change updates all go modules, including oidc, a major version of go-jose and the go 1.22 release.
* Revert "chore(deps): update all go deps" (#7772)
Revert "chore(deps): update all go deps (#7764)"
This reverts commit 6893e7d060.
---------
Co-authored-by: Livio Spring <livio.a@gmail.com>
(cherry picked from commit 9ccbbe05bc)
* fix(middleware): init translation messages
* revert change
* refactor: split loop in separate function
* add imports to ensure init of fs
(cherry picked from commit 9bcfa12be2)
* fix(oidc): roles in userinfo for client credentials token
When tokens were obtained using the client credentials grant,
with audience and role scopes, userinfo would not return the role claims. This had multiple causes:
1. There is no auth request flow, so for legacy userinfo project data was never attached to the token
2. For optimized userinfo, there is no client ID that maps to an application. The client ID for client credentials is the machine user's name. There we can't obtain a project ID. When the project ID remained empty, we always ignored the roleAudience.
This PR fixes situation 2, by always taking the roleAudience into account, even when the projectID is empty. The code responsible for the bug is also refactored to be more readable and understandable, including additional godoc.
The fix only applies to the optimized userinfo code introduced in #7706 and released in v2.50 (currently in RC). Therefore it can't be back-ported to earlier versions.
Fixes#6662
* chore(deps): update all go deps (#7764)
This change updates all go modules, including oidc, a major version of go-jose and the go 1.22 release.
* Revert "chore(deps): update all go deps" (#7772)
Revert "chore(deps): update all go deps (#7764)"
This reverts commit 6893e7d060.
---------
Co-authored-by: Livio Spring <livio.a@gmail.com>
This fixes the projection of events that have a null audience or scope.
As audience was added in v2.50, legacy events do not have an audience, this made replay of the old events not possible after an upgrade.
(cherry picked from commit be00e3861a)
This fixes the projection of events that have a null audience or scope.
As audience was added in v2.50, legacy events do not have an audience, this made replay of the old events not possible after an upgrade.
* feat: smtp templates poc
* feat: add isActive & ProviderType to SMTP backend
* feat: change providertype to uint32 and fix tests
* feat: minimal smtp provider component
* feat: woking on diiferent providers
* feat: keep working on providers
* feat: initial stepper for new provider
* fix: settings list and working on stepper
* feat: step 1 and 2 form inputs
* feat: starter for smtp test step
* fix: misspelled SMPT
* fix: remove tests for now
* feat: add tls toggle remove old google provider
* feat: working on add smtp and table
* fix: duplicated identifiers
* fix: settings list
* fix: add missing smtp config properties
* fix: add configID to smtp config table
* fix: working on listproviders
* feat: working in listSMTPConfigs
* fix: add count to listsmtpconfigs
* fix: getting empty results from listSMTPConfigs
* feat: table now shows real data
* fix: remaining styles for smtp-table
* fix: remove old notification-smtp-provider-component
* feat: delete smtp configuration
* feat: deactivate smtp config
* feat: replace isActive with state for smtp config
* feat: activate smtp config
* fix: remaining errors after main merge
* fix: list smtp providers panic and material mdc
* feat: refactor to only one provider component
* feat: current provider details view
* fix: refactor AddSMTPConfig and ChangeSMTPConfig
* fix: smtp config reduce issue
* fix: recover domain in NewIAMSMTPConfigWriteModel
* fix: add code needed by SetUpInstance
* fix: go tests and warn about passing context to InstanceAggregateFromWriteModel
* fix: i18n and add missing trans for fr, it, zh
* fix: add e2e tests
* docs: add smtp templates
* fix: remove provider_type, add description
* fix: remaining error from merge main
* fix: add @stebenz change for primary key
* fix: inactive placed after removed to prevent deleted configs to show as inactive
* fix: smtp provider id can be empty (migrated)
* feat: add mailchimp transactional template
* feat: add Brevo (Sendinblue) template
* feat: change brevo logo, add color to tls icon
* fix: queries use resourceowner, id must not be empty
* fix: deal with old smtp settings and tests
* fix: resourceOwner is the instanceID
* fix: remove aggregate_id, rename SMTPConfigByAggregateID with SMTPConfigActive
* fix: add tests for multiple configs with different IDs
* fix: conflict
* fix: remove notification-smtp-provider
* fix: add @peintnermax suggestions, rename module and fix e2e tests
* fix: remove material legacy modules
* fix: remove ctx as parameter for InstanceAggregateFromWriteModel
* fix: add Id to SMTPConfigToPb
* fix: change InstanceAggregateFromWriteModel to avoid linter errors
* fix import
* rm unused package-lock
* update yarn lock
---------
Co-authored-by: Elio Bischof <elio@zitadel.com>
Co-authored-by: Max Peintner <max@caos.ch>
Co-authored-by: Stefan Benz <46600784+stebenz@users.noreply.github.com>
* init auto linking
* prompt handling
* working
* translations
* console
* fixes
* unify
* custom texts
* fix tests
* linting
* fix check of existing user
* fix bg translation
* set unspecified as default in the form
* fix: add action v2 execution to features
* fix: add action v2 execution to features
* fix: add action v2 execution to features
* fix: update internal/command/instance_features_model.go
Co-authored-by: Tim Möhlmann <tim+github@zitadel.com>
* fix: merge back main
* fix: merge back main
* fix: rename feature and service
* fix: rename feature and service
* fix: review changes
* fix: review changes
---------
Co-authored-by: Tim Möhlmann <tim+github@zitadel.com>
* feat(oidc): optimize the userinfo endpoint
* store project ID in the access token
* query for projectID if not in token
* add scope based tests
* Revert "store project ID in the access token"
This reverts commit 5f0262f239.
* query project role assertion
* use project role assertion setting to return roles
* workaround eventual consistency and handle PAT
* do not append empty project id
feat(db): wrap BeginTx in spans to get acquire metrics
This changes adds a span around most db.BeginTx calls so we can get tracings about the connection pool acquire process.
This might help us pinpoint why sometimes some query package traces show longer execution times, while this was not reflected on database side execution times.
Co-authored-by: Silvan <silvan.reusser@gmail.com>
(cherry picked from commit 093dd57a78)
* fix(oidc): return bad request for base64 errors
We've recently noticed an increased amount of 500: internal server error status returns on zitadel cloud.
The source of these errors appear to be erroneous input in fields that are supposed to be bas64 formatted.
```
time=2024-04-08T14:05:47.600Z level=ERROR msg="request error" oidc_error.parent="ID=OIDC-AhX2u Message=Errors.Internal Parent=(illegal base64 data at input byte 8)" oidc_error.description=Errors.Internal oidc_error.type=server_error status_code=500
```
Within the possible code paths of the token endpoint there are a couple of uses of base64.Encoding.DecodeString of which a returned error was not properly wrapped, but returned as-is.
This causes the oidc error handler to return a 500 with the `OIDC-AhX2u` ID.
We were not able to pinpoint the exact errors that are happening to any one call of `DecodeString`.
This fix wraps all errors from `DecodeString` so that proper 400: bad request is returned with information about the error. Each wrapper now has an unique error ID, so that logs will contain the source of the error as well.
This bug was reported internally by the ops team.
* catch op.ErrInvalidRefreshToken
(cherry picked from commit c8e0b30e17)
When creating an app without secret or other type of authentication method,
like JWT, and the authentication type is switched afterwards the app would remain without generated secret.
If then client authentication with secret is attempted, for example on the token endpoint, the handler would panic in the crypto.CompareHash function on the nile pointer to the CryptoValue.
This fix introduces a nil pointer check in crypt.CompareHash and returns a error.
The issue was reported over discord: https://discord.com/channels/927474939156643850/1222971118730875020
Possible fix was suggested here: https://github.com/zitadel/zitadel/pull/6999#discussion_r1553503088
This bug only applies to zitadel versions <=2.49.1.
* fix(oidc): return bad request for base64 errors
We've recently noticed an increased amount of 500: internal server error status returns on zitadel cloud.
The source of these errors appear to be erroneous input in fields that are supposed to be bas64 formatted.
```
time=2024-04-08T14:05:47.600Z level=ERROR msg="request error" oidc_error.parent="ID=OIDC-AhX2u Message=Errors.Internal Parent=(illegal base64 data at input byte 8)" oidc_error.description=Errors.Internal oidc_error.type=server_error status_code=500
```
Within the possible code paths of the token endpoint there are a couple of uses of base64.Encoding.DecodeString of which a returned error was not properly wrapped, but returned as-is.
This causes the oidc error handler to return a 500 with the `OIDC-AhX2u` ID.
We were not able to pinpoint the exact errors that are happening to any one call of `DecodeString`.
This fix wraps all errors from `DecodeString` so that proper 400: bad request is returned with information about the error. Each wrapper now has an unique error ID, so that logs will contain the source of the error as well.
This bug was reported internally by the ops team.
* catch op.ErrInvalidRefreshToken
* update url to redirect to name change url
* Update register_option.html
* Update register_option.html
---------
Co-authored-by: Livio Spring <livio.a@gmail.com>
* fix(idp): do not call userinfo when mapping from ID token is configured
This change prevents the call of the Userinfo endpoint of a OIDC IDP if the IDP is configured to use the ID token for user information instead.
A unit test has been added to confirm the corrected behavior.
Closes#7373
* video for e2e
---------
Co-authored-by: Livio Spring <livio.a@gmail.com>
chore(fmt): run gci on complete project
Fix global import formatting in go code by running the `gci` command. This allows us to just use the command directly, instead of fixing the import order manually for the linter, on each PR.
Co-authored-by: Elio Bischof <elio@zitadel.com>
feat(db): wrap BeginTx in spans to get acquire metrics
This changes adds a span around most db.BeginTx calls so we can get tracings about the connection pool acquire process.
This might help us pinpoint why sometimes some query package traces show longer execution times, while this was not reflected on database side execution times.
Co-authored-by: Silvan <silvan.reusser@gmail.com>
* fix: add resource owner as query for user v2 ListUsers and clean up deprecated attribute
* fix: add resource owner as query for user v2 ListUsers and clean up deprecated attribute
* fix: add resource owner as query for user v2 ListUsers and clean up deprecated attribute
* fix: review changes
* fix: review changes
* fix: review changes
* fix: review changes
* fix: add password change required to user v2 get and list
* fix: update unit tests for query side with new column and projection
* fix: change projection in setup steps
* fix: change projection in setup steps
* fix: remove setup step 25
* fix: add password_change_required into ListUsers response
* fix: correct SetUserPassword parameters
* fix: rollback to change setup instead of projection directly
* fix: rollback to change setup instead of projection directly
---------
Co-authored-by: Livio Spring <livio.a@gmail.com>
* fix: remove resourceowner read from context in user v2 api
* fix: lint
* fix: remove orgID in addIDPLink
* fix: remove comment as unnecessary
---------
Co-authored-by: Livio Spring <livio.a@gmail.com>
* chore: use pgx v5
* chore: update go version
* remove direct pq dependency
* remove unnecessary type
* scan test
* map scanner
* converter
* uint8 number array
* duration
* most unit tests work
* unit tests work
* chore: coverage
* go 1.21
* linting
* int64 gopfertammi
* retry go 1.22
* retry go 1.22
* revert to go v1.21.5
* update go toolchain to 1.21.8
* go 1.21.8
* remove test flag
* go 1.21.5
* linting
* update toolchain
* use correct array
* use correct array
* add byte array
* correct value
* correct error message
* go 1.21 compatible
