Skip to content

chore: add harness test for auth enabled - #3728

Merged
akshaydeo merged 2 commits into
devfrom
05-25-chore_add_harness_test_for_auth_enabled
May 25, 2026
Merged

chore: add harness test for auth enabled#3728
akshaydeo merged 2 commits into
devfrom
05-25-chore_add_harness_test_for_auth_enabled

Conversation

@roroghost17

@roroghost17 roroghost17 commented May 25, 2026

Copy link
Copy Markdown
Contributor

Summary

Extends the /api/is-auth-enabled endpoint contract to expose has_valid_token and auth_type fields alongside the existing is_auth_enabled field, and adds E2E test coverage for the new mcp_enable_temp_token_auth config flag.

Changes

  • Updated the Check If Auth Enabled response field assertion to include has_valid_token and auth_type in addition to is_auth_enabled.
  • Added a test script to the is-auth-enabled request that validates:
    • has_valid_token is a boolean
    • auth_type is one of 'none', 'password', or 'sso'
    • When auth is disabled, auth_type is 'none' and has_valid_token is false
    • When auth is enabled, auth_type is not 'none'
  • Added two new Postman requests under the config section:
    • Update Config (Enable Temp Token Auth) — PUTs mcp_enable_temp_token_auth: true to /api/config
    • Get Config (Verify Temp Token Auth) — GETs /api/config and asserts mcp_enable_temp_token_auth round-trips as true

Type of change

  • Bug fix
  • Feature
  • Refactor
  • Documentation
  • Chore/CI

Affected areas

  • Core (Go)
  • Transports (HTTP)
  • Providers/Integrations
  • Plugins
  • UI (React)
  • Docs

How to test

Run the updated Postman collection against a running Bifrost instance:

newman run tests/e2e/api/collections/bifrost-api-management.postman_collection.json \
  --env-var base_url=http://localhost:<port>

Expected outcomes:

  • Check If Auth Enabled response includes is_auth_enabled, has_valid_token, and auth_type
  • auth_type is one of none, password, or sso
  • mcp_enable_temp_token_auth can be set to true via PUT and is returned as true on subsequent GET

Breaking changes

  • Yes
  • No

Related issues

Security considerations

The has_valid_token and auth_type fields are exposed on the is-auth-enabled endpoint. This endpoint is intended to be publicly accessible for client bootstrapping. Care should be taken to ensure no sensitive session or token details are leaked beyond the boolean and enum values validated here.

Checklist

  • I read docs/contributing/README.md and followed the guidelines
  • I added/updated tests where appropriate
  • I updated documentation where needed
  • I verified builds succeed (Go and UI)
  • I verified the CI pipeline passes locally if applicable

@coderabbitai

coderabbitai Bot commented May 25, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Summary by CodeRabbit

  • Tests
    • Updated test collection to verify authentication configuration and temporary token authentication capabilities with expanded response validation.

Walkthrough

The Postman E2E collection is updated to test temporary token authentication configuration. The response contract for the auth check endpoint now requires has_valid_token and auth_type fields alongside is_auth_enabled. New /api/config tests enable and verify the mcp_enable_temp_token_auth configuration, and assertions validate the expanded auth endpoint response with conditional field value expectations.

Changes

Temp Token Auth E2E Tests

Layer / File(s) Summary
Auth check response contract and assertions
tests/e2e/api/collections/bifrost-api-management.postman_collection.json
Response contract updated to validate has_valid_token and auth_type in addition to is_auth_enabled. Assertions added to the Session -> Check If Auth Enabled test enforce field existence, types, and conditional value expectations based on auth enabled state.
Config endpoint tests for temp token auth
tests/e2e/api/collections/bifrost-api-management.postman_collection.json
New /api/config test flows added: Update Config (Enable Temp Token Auth) sends mcp_enable_temp_token_auth: true and validates acceptance; Get Config (Verify Temp Token Auth) asserts the configuration value round-trips to true.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested reviewers

  • danpiths
  • akshaydeo

Poem

🐰 Temp tokens dance in configs bright,
New assertions verify the sight,
Response fields bloom in auth's embrace,
Postman tests now set the pace! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive The PR title 'chore: add harness test for auth enabled' is partially related to the changeset—it references test additions but obscures the primary change of extending the /api/is-auth-enabled endpoint response contract. Consider a more specific title like 'chore: add auth endpoint response validation and temp token auth config tests' to better reflect the scope of changes.
✅ Passed checks (4 passed)
Check name Status Explanation
Description check ✅ Passed The PR description is comprehensive, well-structured, and follows the template with all major sections completed including summary, changes, type, affected areas, testing instructions, and security considerations.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch 05-25-chore_add_harness_test_for_auth_enabled

Comment @coderabbitai help to get the list of available commands and usage tips.

@CLAassistant

CLAassistant commented May 25, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

roroghost17 commented May 25, 2026

Copy link
Copy Markdown
Contributor Author

@roroghost17
roroghost17 marked this pull request as ready for review May 25, 2026 11:25
@coderabbitai
coderabbitai Bot requested review from akshaydeo and danpiths May 25, 2026 11:26
@greptile-apps

greptile-apps Bot commented May 25, 2026

Copy link
Copy Markdown
Contributor

Confidence Score: 3/5

Safe to merge on an isolated CI instance, but will leave mcp_enable_temp_token_auth permanently enabled on any shared or long-lived server it runs against.

The only changed file is a Postman collection, so no production code is affected. The functional concern is that the PUT to enable mcp_enable_temp_token_auth has no matching teardown, so repeated runs or parallel environments will inherit a mutated config state. The is-auth-enabled test logic itself is correct and well-structured.

tests/e2e/api/collections/bifrost-api-management.postman_collection.json — the missing reset step after the temp-token-auth PUT.

Important Files Changed

Filename Overview
tests/e2e/api/collections/bifrost-api-management.postman_collection.json Adds has_valid_token/auth_type field assertions to the is-auth-enabled request, two new config PUT/GET requests for mcp_enable_temp_token_auth, and expands the global response-shape map; the new PUT leaves mcp_enable_temp_token_auth: true permanently with no teardown.

Reviews (1): Last reviewed commit: "chore: add harness test for auth enabled" | Re-trigger Greptile

@coderabbitai coderabbitai 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.

🧹 Nitpick comments (2)
tests/e2e/api/collections/bifrost-api-management.postman_collection.json (2)

2314-2321: ⚡ Quick win

Assert is_auth_enabled is a boolean explicitly.

You already validate type for has_valid_token; doing the same for is_auth_enabled tightens this contract test.

Proposed diff
   pm.expect(body).to.have.property('is_auth_enabled');
   pm.expect(body).to.have.property('has_valid_token');
   pm.expect(body).to.have.property('auth_type');
+  pm.expect(body.is_auth_enabled, 'is_auth_enabled is boolean').to.be.a('boolean');
   pm.expect(body.has_valid_token, 'has_valid_token is boolean').to.be.a('boolean');
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/e2e/api/collections/bifrost-api-management.postman_collection.json`
around lines 2314 - 2321, The contract test "is-auth-enabled exposes auth_type +
has_valid_token contract" currently asserts types for has_valid_token and
auth_type but not for is_auth_enabled; update the Postman test block (the
pm.test with that name) to add an explicit type assertion for
body.is_auth_enabled (e.g., pm.expect(body.is_auth_enabled, 'is_auth_enabled is
boolean').to.be.a('boolean')) so is_auth_enabled is validated as a boolean
alongside has_valid_token and auth_type.

444-535: ⚡ Quick win

Wire the new config requests into global response-shape checks.

These new request names skip the collection-level successShapes contract gate, so only local assertions run. Add shape mappings for them to keep stack-wide contract coverage consistent.

Proposed diff
   'Get Config': ['auth_config', 'client_config', 'framework_config', 'is_db_connected', 'is_logs_connected'],
   'Update Config': ['message', 'status'],
+  'Get Config (Verify Temp Token Auth)': ['auth_config', 'client_config', 'framework_config', 'is_db_connected', 'is_logs_connected'],
+  'Update Config (Enable Temp Token Auth)': ['message', 'status'],
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/e2e/api/collections/bifrost-api-management.postman_collection.json`
around lines 444 - 535, The two new Postman requests "Update Config (Enable Temp
Token Auth)" and "Get Config (Verify Temp Token Auth)" bypass the
collection-level successShapes contract; add mappings for these request names
into the collection's successShapes response-shape map so they run the global
shape checks. Locate the collection-level successShapes mapping (the JSON object
that maps request names to shape IDs) and add entries for "Update Config (Enable
Temp Token Auth)" pointing to the appropriate update-config success shape and
"Get Config (Verify Temp Token Auth)" pointing to the config-get success shape
(reuse existing config-related shape IDs used by other Config requests) so the
global contract gate covers these requests.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@tests/e2e/api/collections/bifrost-api-management.postman_collection.json`:
- Around line 2314-2321: The contract test "is-auth-enabled exposes auth_type +
has_valid_token contract" currently asserts types for has_valid_token and
auth_type but not for is_auth_enabled; update the Postman test block (the
pm.test with that name) to add an explicit type assertion for
body.is_auth_enabled (e.g., pm.expect(body.is_auth_enabled, 'is_auth_enabled is
boolean').to.be.a('boolean')) so is_auth_enabled is validated as a boolean
alongside has_valid_token and auth_type.
- Around line 444-535: The two new Postman requests "Update Config (Enable Temp
Token Auth)" and "Get Config (Verify Temp Token Auth)" bypass the
collection-level successShapes contract; add mappings for these request names
into the collection's successShapes response-shape map so they run the global
shape checks. Locate the collection-level successShapes mapping (the JSON object
that maps request names to shape IDs) and add entries for "Update Config (Enable
Temp Token Auth)" pointing to the appropriate update-config success shape and
"Get Config (Verify Temp Token Auth)" pointing to the config-get success shape
(reuse existing config-related shape IDs used by other Config requests) so the
global contract gate covers these requests.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f5f54466-cf9d-468d-a618-2bfdd755c881

📥 Commits

Reviewing files that changed from the base of the PR and between 737f4d1 and 46b7eb2.

📒 Files selected for processing (1)
  • tests/e2e/api/collections/bifrost-api-management.postman_collection.json

coderabbitai[bot]
coderabbitai Bot previously approved these changes May 25, 2026
akshaydeo
akshaydeo previously approved these changes May 25, 2026

akshaydeo commented May 25, 2026

Copy link
Copy Markdown
Contributor

Merge activity

  • May 25, 4:37 PM UTC: A user started a stack merge that includes this pull request via Graphite.
  • May 25, 4:38 PM UTC: Graphite couldn't merge this PR because it was not satisfying all requirements.
  • May 25, 4:40 PM UTC: A user started a stack merge that includes this pull request via Graphite.
  • May 25, 4:40 PM UTC: @akshaydeo merged this pull request with Graphite.

@akshaydeo
akshaydeo changed the base branch from 05-23-fix_allows_for_toggling_non-auth_users_for_mcp_temp_tokens to graphite-base/3728 May 25, 2026 16:37
@akshaydeo
akshaydeo changed the base branch from graphite-base/3728 to dev May 25, 2026 16:37
@akshaydeo
akshaydeo dismissed stale reviews from coderabbitai[bot] and themself May 25, 2026 16:37

The base branch was changed.

@akshaydeo
akshaydeo requested a review from a team as a code owner May 25, 2026 16:37
@akshaydeo
akshaydeo merged commit f567f52 into dev May 25, 2026
9 of 10 checks passed
@akshaydeo
akshaydeo deleted the 05-25-chore_add_harness_test_for_auth_enabled branch May 25, 2026 16:40
akshaydeo pushed a commit that referenced this pull request May 26, 2026
## Summary

Extends the `/api/is-auth-enabled` endpoint contract to expose `has_valid_token` and `auth_type` fields alongside the existing `is_auth_enabled` field, and adds E2E test coverage for the new `mcp_enable_temp_token_auth` config flag.

## Changes

- Updated the `Check If Auth Enabled` response field assertion to include `has_valid_token` and `auth_type` in addition to `is_auth_enabled`.
- Added a test script to the `is-auth-enabled` request that validates:
  - `has_valid_token` is a boolean
  - `auth_type` is one of `'none'`, `'password'`, or `'sso'`
  - When auth is disabled, `auth_type` is `'none'` and `has_valid_token` is `false`
  - When auth is enabled, `auth_type` is not `'none'`
- Added two new Postman requests under the config section:
  - **Update Config (Enable Temp Token Auth)** — PUTs `mcp_enable_temp_token_auth: true` to `/api/config`
  - **Get Config (Verify Temp Token Auth)** — GETs `/api/config` and asserts `mcp_enable_temp_token_auth` round-trips as `true`

## Type of change

- [ ] Bug fix
- [x] Feature
- [ ] Refactor
- [ ] Documentation
- [ ] Chore/CI

## Affected areas

- [ ] Core (Go)
- [x] Transports (HTTP)
- [ ] Providers/Integrations
- [ ] Plugins
- [ ] UI (React)
- [ ] Docs

## How to test

Run the updated Postman collection against a running Bifrost instance:

```sh
newman run tests/e2e/api/collections/bifrost-api-management.postman_collection.json \
  --env-var base_url=http://localhost:<port>
```

Expected outcomes:
- `Check If Auth Enabled` response includes `is_auth_enabled`, `has_valid_token`, and `auth_type`
- `auth_type` is one of `none`, `password`, or `sso`
- `mcp_enable_temp_token_auth` can be set to `true` via PUT and is returned as `true` on subsequent GET

## Breaking changes

- [ ] Yes
- [x] No

## Related issues

## Security considerations

The `has_valid_token` and `auth_type` fields are exposed on the `is-auth-enabled` endpoint. This endpoint is intended to be publicly accessible for client bootstrapping. Care should be taken to ensure no sensitive session or token details are leaked beyond the boolean and enum values validated here.

## Checklist

- [ ] I read `docs/contributing/README.md` and followed the guidelines
- [x] I added/updated tests where appropriate
- [ ] I updated documentation where needed
- [ ] I verified builds succeed (Go and UI)
- [ ] I verified the CI pipeline passes locally if applicable
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants