authgear / authgear/authgear-server
Unmatched input / output of authgear config json
- Dominant language
- Go
- Stars
- 2k
- Forks
- 125
- Avg merge
- 2d 17h
- Merged PRs (30d)
- 32
Description
This is an issue discovered in https://github.com/authgear/authgear-server/issues/3106
## Description
Lets use the `tenant` field in [OAuthSSOProviderConfig](https://github.com/authgear/authgear-server/blob/2023C01/pkg/lib/config/identity.go#L393) as an example.
If we now update the config using the `updateApp` mutation, with the following input:
```jsonc
{
... // skip other configs because they are not important here
"providers: [{
... // skip other fields
"type": "azureadv2",
"tenant": ""
}]
}
```
The above input will pass the json schema validation, because `tenant` is not null.
Then, from the same graphql, we got the following output:
```jsonc
{
... // skip other configs because they are not important here
"providers: [{
... // skip other fields
"type": "azureadv2"
// <--- No "tenant" field
}]
}
```
The `tenant` field will be removed from the output, because empty string is the zero value of `string` in golang, and we've specified `omitempty` in the json.
And note that, the above output **will fail** the input validation because `tenant` is required, but was not specified.
## Solutions
1. Review all validation rules of json schema and add back necessary "minLength: 1" to strings.
2. Remove `omitempty` in required fields. Use pointer types `*string` / `*bool` if that field does not always exist.
Contributor guide
Research direction
Start with pkg/lib/config/identity.go, especially OAuthSSOProviderConfig and its tenant field, then trace the updateApp mutation's JSON schema validation and serialization. Compare required fields and omitempty behavior across the affected configuration types; done means valid updateApp input remains valid when returned by the same GraphQL API, with regression coverage for the round trip.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- authentication, backend-api-design
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 38/100