Skip to content

feat: Add OAuth support for HTTP Sync#1791

Merged
toddbaert merged 16 commits into
open-feature:mainfrom
thisthat:1785-add-oauth
Oct 6, 2025
Merged

feat: Add OAuth support for HTTP Sync#1791
toddbaert merged 16 commits into
open-feature:mainfrom
thisthat:1785-add-oauth

Conversation

@thisthat

@thisthat thisthat commented Sep 2, 2025

Copy link
Copy Markdown
Member

This PR

  • adds support for OAuth in the HTTP Sync

Related Issues

Addresses #1785

Notes

Properly testing this via unit tests is not possible. The HTTP Sync is built by the syncbuilder pkg. This entails that testing any behavior of the HTTP Sync can only be possible as part of the syncbuilder module, which IMHO doesn't make much sense.

Follow-up Tasks

Add documentation for the new properties used in the HTTP Sync.
Available here: #1805

How to test

Manually create a small mockup server that handles OAuth requests

func (h *OAuthHandler) Authorize(w http.ResponseWriter, r *http.Request) {

	reqDump, _ := httputil.DumpRequest(r, true)
	fmt.Printf("GOT OAUTH REQUEST:\n%s", string(reqDump))

	w.WriteHeader(http.StatusOK)
	w.Header().Set("Content-Type", "application/x-www-form-urlencoded")
	w.Write([]byte("access_token=mocktoken&scope=mockscope&token_type=bearer"))
}

Afterwards, start flagd pointing to the mockup server:

go run main.go start --sources='[{ "uri": "http://localhost:8180/flags", "provider": "http", "interval": 1, "bearerToken": "it-will-be-overridden", "oauthConfig": { "clientID": "test", "clientSecret": "test", "tokenUrl": "http://localhost:8180/oauth" }}]'

The HTTP request to http://localhost:8180/flags will contain the HTTP Header Authorization: Bearer mocktoken.

@thisthat thisthat requested review from a team as code owners September 2, 2025 19:07
@netlify

netlify Bot commented Sep 2, 2025

Copy link
Copy Markdown

Deploy Preview for polite-licorice-3db33c canceled.

Name Link
🔨 Latest commit d0b41ee
🔍 Latest deploy log https://app.netlify.com/projects/polite-licorice-3db33c/deploys/68e3e32a7b9b45000833f23d

@dosubot dosubot Bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Sep 2, 2025

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary of Changes

Hello @thisthat, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request introduces OAuth 2.0 client credentials flow support for HTTP synchronization, allowing flagd to authenticate with remote flag sources using OAuth. This enhances security and flexibility for fetching flag configurations.

Highlights

  • OAuth Integration: Implemented OAuth 2.0 client credentials flow for HTTP Sync, enabling secure authentication with flag sources.
  • Configurable HTTP Client: The HTTP client used for synchronization now dynamically configures itself to use OAuth if oauthConfig is provided in the source configuration.
  • New Configuration Structure: Introduced a dedicated OAuthConfig structure to define OAuth client ID, client secret, and token URL within the source configuration.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in issue comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces OAuth support for the HTTP Sync provider, which is a great addition. The implementation is solid, but I've identified a few areas for improvement to enhance robustness, flexibility, and adherence to Go's idiomatic coding style. Specifically, the new OAuth HTTP client is missing a request timeout, which could lead to hanging requests. Additionally, the OAuth authentication style is hardcoded, which might limit compatibility with some OAuth servers. Lastly, some of the new struct field names could be adjusted to better align with Go conventions. I've provided detailed comments and code suggestions to address these points.

Comment thread core/pkg/sync/builder/syncbuilder.go Outdated
Comment thread core/pkg/sync/builder/utils_test.go Outdated
Comment thread core/pkg/sync/isync.go Outdated
@dosubot dosubot Bot added size:L This PR changes 100-499 lines, ignoring generated files. and removed size:M This PR changes 30-99 lines, ignoring generated files. labels Sep 3, 2025
Comment thread core/pkg/sync/http/http_sync.go Outdated
@thisthat thisthat marked this pull request as draft September 3, 2025 06:30
@thisthat thisthat marked this pull request as ready for review September 4, 2025 11:54
@dosubot dosubot Bot added size:XL This PR changes 500-999 lines, ignoring generated files. and removed size:L This PR changes 100-499 lines, ignoring generated files. labels Sep 4, 2025
), nil
}

func (sb *SyncBuilder) newHTTP(config sync.SourceConfig, logger *logger.Logger) *httpSync.Sync {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

move the logic of creating the HTTPSync inside the HTTPSync

BearerToken string
AuthHeader string
Interval uint32
uri string

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we don't need to expose these details anymore since we moved its construction here

Comment thread core/pkg/sync/http/http_sync.go Outdated

@aepfli aepfli left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally those changes look great, we do have some minor improvements/questions - but generally the implementation looks fine.

The biggest part missing tough is documentation, and currently this looks like a hidden feature. Especially the fact that filesource for oauth takes priority, and the current double purpose of clientId and clientSecret.

Comment thread core/pkg/sync/http/http_sync.go Outdated
Comment thread core/pkg/sync/http/http_sync.go
Comment thread core/pkg/sync/http/http_sync.go
Comment thread core/pkg/sync/isync.go Outdated
@toddbaert

Copy link
Copy Markdown
Member

I'll review this tomorrow, first thing. Thanks @thisthat

renovate Bot and others added 10 commits September 29, 2025 06:57
…2.4.0 [security] (open-feature#1784)

This PR contains the following updates:

| Package | Change | Age | Confidence |
|---|---|---|---|
|
[github.com/go-viper/mapstructure/v2](https://redirect.github.com/go-viper/mapstructure)
| `v2.3.0` -> `v2.4.0` |
[![age](https://developer.mend.io/api/mc/badges/age/go/github.com%2fgo-viper%2fmapstructure%2fv2/v2.4.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/go/github.com%2fgo-viper%2fmapstructure%2fv2/v2.3.0/v2.4.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

### GitHub Vulnerability Alerts

####
[GHSA-2464-8j7c-4cjm](https://redirect.github.com/go-viper/mapstructure/security/advisories/GHSA-2464-8j7c-4cjm)

### Summary

Use of this library in a security-critical context may result in leaking
sensitive information, if used to process sensitive fields.

### Details

OpenBao (and presumably HashiCorp Vault) have surfaced error messages
from `mapstructure` as follows:

https://github.com/openbao/openbao/blob/98c3a59c040efca724353ca46ca79bd5cdbab920/sdk/framework/field_data.go#L43-L50

```go
			_, _, err := d.getPrimitive(field, schema)
			if err != nil {
				return fmt.Errorf("error converting input for field %q: %w", field, err)
			}
```

where this calls `mapstructure.WeakDecode(...)`:
https://github.com/openbao/openbao/blob/98c3a59c040efca724353ca46ca79bd5cdbab920/sdk/framework/field_data.go#L181-L193

```go

func (d *FieldData) getPrimitive(k string, schema *FieldSchema) (interface{}, bool, error) {
	raw, ok := d.Raw[k]
	if !ok {
		return nil, false, nil
	}

	switch t := schema.Type; t {
	case TypeBool:
		var result bool
		if err := mapstructure.WeakDecode(raw, &result); err != nil {
			return nil, false, err
		}
		return result, true, nil
```

Notably, `WeakDecode(...)` eventually calls one of the decode helpers,
which surfaces the original value via `strconv` helpers:

https://github.com/go-viper/mapstructure/blob/8c61ec1924fcfa522f9fc6b4618c672db61d1a38/mapstructure.go#L720-L727

https://github.com/go-viper/mapstructure/blob/8c61ec1924fcfa522f9fc6b4618c672db61d1a38/mapstructure.go#L791-L798

https://github.com/go-viper/mapstructure/blob/8c61ec1924fcfa522f9fc6b4618c672db61d1a38/decode_hooks.go#L180

& more. These are different code paths than are fixed in the previous
iteration at
GHSA-fv92-fjc5-jj9h.

### PoC

To reproduce with OpenBao:

```
$ podman run --pull=always -p 8300:8300 openbao/openbao:latest server -dev -dev-root-token-id=root -dev-listen-address=0.0.0.0:8300
```

and in a new tab:

```
$ BAO_TOKEN=root BAO_ADDR=http://localhost:8300 bao auth enable userpass
Success! Enabled userpass auth method at: userpass/
$ curl -X PUT -H "X-Vault-Request: true" -H "X-Vault-Token: root" -d '{"ttl":"asdf"}' "http://localhost:8200/v1/auth/userpass/users/asdf"

--> server logs:

2025-06-25T21:32:25.101-0500 [ERROR] core: failed to run existence check: error="error converting input for field \"ttl\": time: invalid duration \"asdf\""
```

### Impact

This is an information disclosure bug with little mitigation. See
https://discuss.hashicorp.com/t/hcsec-2025-09-vault-may-expose-sensitive-information-in-error-logs-when-processing-malformed-data-with-the-kv-v2-plugin/74717
for a previous version. That version was fixed, but this is in the
second part of that error message (starting at `'' expected a map, got
'string'` -- when the field type is `string` and a `map` is provided, we
see the above information leak -- the previous example had a `map` type
field with a `string` value provided).

This was rated 4.5 Medium by HashiCorp in the past iteration.

---

### Release Notes

<details>
<summary>go-viper/mapstructure
(github.com/go-viper/mapstructure/v2)</summary>

###
[`v2.4.0`](https://redirect.github.com/go-viper/mapstructure/releases/tag/v2.4.0)

[Compare
Source](https://redirect.github.com/go-viper/mapstructure/compare/v2.3.0...v2.4.0)

#### What's Changed

- refactor: replace interface{} with any by
[@&#8203;sagikazarmark](https://redirect.github.com/sagikazarmark) in
[https://github.com/go-viper/mapstructure/pull/115](https://redirect.github.com/go-viper/mapstructure/pull/115)
- build(deps): bump github/codeql-action from 3.29.0 to 3.29.2 by
[@&#8203;dependabot](https://redirect.github.com/dependabot)\[bot]
in[https://github.com/go-viper/mapstructure/pull/114](https://redirect.github.com/go-viper/mapstructure/pull/114)4
- Generic tests by
[@&#8203;sagikazarmark](https://redirect.github.com/sagikazarmark) in
[https://github.com/go-viper/mapstructure/pull/118](https://redirect.github.com/go-viper/mapstructure/pull/118)
- Fix godoc reference link in README.md by
[@&#8203;peczenyj](https://redirect.github.com/peczenyj) in
[https://github.com/go-viper/mapstructure/pull/107](https://redirect.github.com/go-viper/mapstructure/pull/107)
- feat: add StringToTimeLocationHookFunc to convert strings to
\*time.Location by
[@&#8203;ErfanMomeniii](https://redirect.github.com/ErfanMomeniii) in
[https://github.com/go-viper/mapstructure/pull/117](https://redirect.github.com/go-viper/mapstructure/pull/117)
- feat: add back previous StringToSlice as a weak function by
[@&#8203;sagikazarmark](https://redirect.github.com/sagikazarmark) in
[https://github.com/go-viper/mapstructure/pull/119](https://redirect.github.com/go-viper/mapstructure/pull/119)

#### New Contributors

- [@&#8203;ErfanMomeniii](https://redirect.github.com/ErfanMomeniii)
made their first contribution in
[https://github.com/go-viper/mapstructure/pull/117](https://redirect.github.com/go-viper/mapstructure/pull/117)

**Full Changelog**:
go-viper/mapstructure@v2.3.0...v2.4.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - "" (UTC), Automerge - At any time (no
schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR is behind base branch, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR was generated by [Mend Renovate](https://mend.io/renovate/).
View the [repository job
log](https://developer.mend.io/github/open-feature/flagd).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0MS44MS4yIiwidXBkYXRlZEluVmVyIjoiNDEuODEuMiIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOlsicmVub3ZhdGUiXX0=-->

---------

Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Todd Baert <todd.baert@dynatrace.com>
Signed-off-by: Giovanni Liva <giovanni.liva@dynatrace.com>
Signed-off-by: Giovanni Liva <giovanni.liva@dynatrace.com>
Signed-off-by: Giovanni Liva <giovanni.liva@dynatrace.com>
Signed-off-by: Giovanni Liva <giovanni.liva@dynatrace.com>
Signed-off-by: Giovanni Liva <giovanni.liva@dynatrace.com>
Signed-off-by: Giovanni Liva <giovanni.liva@dynatrace.com>
Signed-off-by: Giovanni Liva <giovanni.liva@dynatrace.com>
Signed-off-by: Giovanni Liva <giovanni.liva@dynatrace.com>
Signed-off-by: Giovanni Liva <giovanni.liva@dynatrace.com>
…-feature#1792)

Signed-off-by: Simon Schrottner <simon.schrottner@dynatrace.com>
Signed-off-by: Giovanni Liva <giovanni.liva@dynatrace.com>
aepfli and others added 4 commits September 29, 2025 06:57
Signed-off-by: Simon Schrottner <simon.schrottner@dynatrace.com>
Signed-off-by: Giovanni Liva <giovanni.liva@dynatrace.com>
Signed-off-by: Simon Schrottner <simon.schrottner@dynatrace.com>
Co-authored-by: chrfwow <christian.lutnik@dynatrace.com>
Signed-off-by: Giovanni Liva <giovanni.liva@dynatrace.com>
depends: open-feature#1792 for fixing build issues

---------

Signed-off-by: Simon Schrottner <simon.schrottner@dynatrace.com>
Signed-off-by: Giovanni Liva <giovanni.liva@dynatrace.com>
Signed-off-by: Giovanni Liva <giovanni.liva@dynatrace.com>
@dosubot dosubot Bot added size:XXL This PR changes 1000+ lines, ignoring generated files. and removed size:XL This PR changes 500-999 lines, ignoring generated files. labels Sep 29, 2025
@dosubot dosubot Bot added size:XL This PR changes 500-999 lines, ignoring generated files. and removed size:XXL This PR changes 1000+ lines, ignoring generated files. labels Sep 29, 2025
Signed-off-by: Todd Baert <todd.baert@dynatrace.com>

@toddbaert toddbaert left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I pushed one small change to fix some static checks on a unit test and improve some mocking.

Looks great @thisthat , thanks 🙏

@toddbaert toddbaert merged commit 268fd75 into open-feature:main Oct 6, 2025
19 of 20 checks passed
@github-actions github-actions Bot mentioned this pull request Oct 6, 2025
toddbaert added a commit that referenced this pull request Oct 6, 2025
<!-- Please use this template for your pull request. -->
<!-- Please use the sections that you need and delete other sections -->

## This PR

- adds docs for #1791

### Related Issues

Fixes #1785

---------

Signed-off-by: Giovanni Liva <giovanni.liva@dynatrace.com>
Signed-off-by: Simon Schrottner <simon.schrottner@dynatrace.com>
Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
Co-authored-by: Simon Schrottner <simon.schrottner@dynatrace.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: alexandraoberaigner <82218944+alexandraoberaigner@users.noreply.github.com>
Co-authored-by: Todd Baert <todd.baert@dynatrace.com>
toddbaert added a commit that referenced this pull request Dec 24, 2025
🤖 I have created a release *beep* *boop*
---


<details><summary>flagd: 0.13.0</summary>

##
[0.13.0](flagd/v0.12.9...flagd/v0.13.0)
(2025-12-23)


### 🐛 Bug Fixes

* fixing sync return format missing flag layer, adding full e2e suite
([#1827](#1827))
([570693d](570693d))
* **security:** update module github.com/go-viper/mapstructure/v2 to
v2.4.0 [security]
([#1784](#1784))
([037e30b](037e30b))
* **security:** update module golang.org/x/crypto to v0.45.0 [security]
([#1826](#1826))
([7e0762b](7e0762b))


### ✨ New Features

* add support for http-based ofrep metrics
([#1803](#1803))
([fcd19b3](fcd19b3))
* cleanup evaluator interface
([#1793](#1793))
([aa504f7](aa504f7))
* enable parsing of array flag configurations for flagd
([#1797](#1797))
([97c6ffa](97c6ffa))
* multi-project support via selectors and flagSetId namespacing
([#1702](#1702))
([f9ce46f](f9ce46f))
* normalize selector in sync (use header as in OFREP and RPC)
([#1815](#1815))
([c1f06cb](c1f06cb))


### 🧹 Chore

* **refactor:** use memdb for flag storage
([#1697](#1697))
([5c5c1cf](5c5c1cf))


### 🔄 Refactoring

* store cleanup
([#1705](#1705))
([bcff8d7](bcff8d7))
</details>

<details><summary>flagd-proxy: 0.8.1</summary>

##
[0.8.1](flagd-proxy/v0.8.0...flagd-proxy/v0.8.1)
(2025-12-23)


### 🐛 Bug Fixes

* **security:** update module github.com/go-viper/mapstructure/v2 to
v2.4.0 [security]
([#1784](#1784))
([037e30b](037e30b))
* **security:** update module golang.org/x/crypto to v0.45.0 [security]
([#1826](#1826))
([7e0762b](7e0762b))
</details>

<details><summary>core: 0.13.0</summary>

##
[0.13.0](core/v0.12.1...core/v0.13.0)
(2025-12-23)


### ⚠ BREAKING CHANGES

* enable parsing of array flag configurations for flagd
([#1797](#1797))
* cleanup evaluator interface
([#1793](#1793))
* removes the `fractionalEvaluation` operator since it has been replaced
with `fractional`.
([#1704](#1704))

### 🐛 Bug Fixes

* **security:** update module github.com/go-viper/mapstructure/v2 to
v2.4.0 [security]
([#1784](#1784))
([037e30b](037e30b))
* **security:** update module golang.org/x/crypto to v0.45.0 [security]
([#1825](#1825))
([44edcc9](44edcc9))
* **security:** update module golang.org/x/crypto to v0.45.0 [security]
([#1826](#1826))
([7e0762b](7e0762b))


### ✨ New Features

* Add OAuth support for HTTP Sync
([#1791](#1791))
([268fd75](268fd75))
* Add OTEL default variables
([#1812](#1812))
([c2e3fc6](c2e3fc6))
* allow null flagSetId Selector, restrict Selector to single
key-value-pairs
([#1708](#1708))
([#1811](#1811))
([c12a0ae](c12a0ae))
* change jsonschema parser
([#1794](#1794))
([bf3f722](bf3f722))
* cleanup evaluator interface
([#1793](#1793))
([aa504f7](aa504f7))
* enable parsing of array flag configurations for flagd
([#1797](#1797))
([97c6ffa](97c6ffa))
* multi-project support via selectors and flagSetId namespacing
([#1702](#1702))
([f9ce46f](f9ce46f))


### 🧹 Chore

* **refactor:** use memdb for flag storage
([#1697](#1697))
([5c5c1cf](5c5c1cf))
* removes the `fractionalEvaluation` operator since it has been replaced
with `fractional`.
([#1704](#1704))
([3228ad8](3228ad8))


### 🔄 Refactoring

* remove deprecated bearerToken option
([#1816](#1816))
([efda06a](efda06a))
* removed unused Selector from Flag and Store.
([#1747](#1747))
([1083005](1083005))
* store cleanup
([#1705](#1705))
([bcff8d7](bcff8d7))
</details>

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

---------

Signed-off-by: OpenFeature Bot <109696520+openfeaturebot@users.noreply.github.com>
Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Todd Baert <todd.baert@dynatrace.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XL This PR changes 500-999 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants