Skip to content

Add oauth 2.1 check#37

Open
mooreds wants to merge 54 commits into
mainfrom
mooreds/add-oauth-2-1-check
Open

Add oauth 2.1 check#37
mooreds wants to merge 54 commits into
mainfrom
mooreds/add-oauth-2-1-check

Conversation

@mooreds

@mooreds mooreds commented Mar 27, 2026

Copy link
Copy Markdown
Contributor

This checks against the OAuth 2.1 recommendations for a given FusionAuth installation.

Copilot AI 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.

Pull request overview

Adds a new CLI command to assess a FusionAuth instance against key OAuth 2.1 (draft-ietf-oauth-v2-1-15) recommendations, with both human-readable and JSON output modes.

Changes:

  • Introduces check:oauth-2-1 command to validate tenant-, instance-, and application-level settings relevant to OAuth 2.1 guidance.
  • Updates documentation to describe the new command, its options, and what is checked.
  • Upgrades @fusionauth/typescript-client to support the new checks and updates lockfile accordingly.

Reviewed changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/commands/index.ts Exports the new check:oauth-2-1 command for auto-registration by the CLI.
src/commands/check-oauth-2-1.ts Implements the OAuth 2.1 compliance checking logic and output formatting.
package.json Bumps @fusionauth/typescript-client dependency version/range.
package-lock.json Updates lockfile for new dependency versions and bumps package version.
README.md Documents the new OAuth 2.1 compliance command and its behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/commands/check-oauth-2-1.ts Outdated
Comment thread README.md Outdated
Comment thread package.json Outdated

Copilot AI 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.

Pull request overview

Copilot reviewed 5 out of 6 changed files in this pull request and generated 4 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/utils.ts Outdated
Comment on lines +395 to +400
if (tenantId) {
tenants = tenants.filter(t => t.id === tenantId);
if (tenants.length === 0) {
errorAndExit(`Tenant with ID "${tenantId}" not found.`);
return;
}

Copilot AI Apr 7, 2026

Copy link

Choose a reason for hiding this comment

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

When both --tenant-id and --application-id are provided, the code filters the tenants list but then retrieves and checks the application by ID without validating it belongs to the selected tenant. This can produce a mixed result set (tenant-level checks for one tenant, app-level checks for another). Consider either rejecting the combination as invalid or validating app.tenantId === tenantId and erroring if it doesn’t match.

Copilot uses AI. Check for mistakes.
Comment thread src/commands/check-oauth-2-1.ts Outdated
Comment thread README.md Outdated
- Tenant issuer properly configured — not default "acme.com"

**WARNINGS (informational, does not cause exit 1):**
- DPoP (sender-constrained tokens) enabled on applications (§1.4.3)

Copilot AI Apr 7, 2026

Copy link

Choose a reason for hiding this comment

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

The README states “DPoP … enabled on applications”, but the implementation treats DPoP as an instance-level capability with no per-application toggle (and only checks license/reactor status). Consider rewording this bullet to reflect what the command actually validates (e.g., DPoP availability/Enterprise licensing) to avoid implying an application-level setting exists.

Suggested change
- DPoP (sender-constrained tokens) enabled on applications (§1.4.3)
- DPoP (sender-constrained tokens) available on the FusionAuth instance / Enterprise-enabled (§1.4.3)

Copilot uses AI. Check for mistakes.
mooreds and others added 2 commits April 7, 2026 16:12
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot AI 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.

Pull request overview

Copilot reviewed 5 out of 6 changed files in this pull request and generated 4 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +399 to +407
// Tenants
const tenantResponse = await client.retrieveTenants();
if (!tenantResponse.wasSuccessful() || !tenantResponse.response.tenants) {
errorAndExit('Failed to retrieve tenants.');
return;
}

let tenants = tenantResponse.response.tenants;
if (tenantId) {

Copilot AI Apr 7, 2026

Copy link

Choose a reason for hiding this comment

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

When --application-id is provided without --tenant-id, tenants remains the full tenant list and the tenant-level checks (issuer / refresh token revocation / auth code lifetime) will run against all tenants. This can cause the command to fail due to unrelated tenant settings, even though the user asked to check a single application. Consider narrowing tenants to the application's tenantId when applicationId is set (or skipping tenant-level checks not applicable to the app scope).

Copilot uses AI. Check for mistakes.

// -- Instance-level checks -------------------------------------------

const dpopResult = checkDpop(dpopFeatureActive ?? false);

Copilot AI Apr 7, 2026

Copy link

Choose a reason for hiding this comment

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

dpopFeatureActive has a tri-state (true/false/unknown) but the check currently coerces undefined to false via dpopFeatureActive ?? false, which reports DPoP as unavailable even when the status couldn't be determined (and JSON output has no way to represent the unknown state). Consider propagating an explicit unknown state into the result/output (e.g., passed: null or a distinct message) instead of treating unknown as unavailable.

Suggested change
const dpopResult = checkDpop(dpopFeatureActive ?? false);
const dpopResult = checkDpop(dpopFeatureActive);

Copilot uses AI. Check for mistakes.
Comment thread src/commands/check-oauth-2-1.ts Outdated
Comment thread src/utils.ts
return response.wasSuccessful !== undefined;
export const isClientResponse = (response: any): response is ClientResponseLike => {
return response !== null
&& typeof response === 'object'

Copilot AI Apr 7, 2026

Copy link

Choose a reason for hiding this comment

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

isClientResponse narrows to ClientResponseLike which requires a response property, but the runtime check only verifies wasSuccessful is a function. This makes the type guard unsound (it can return true for objects without response). Consider also checking that a response key exists (and optionally that it's an object) before returning true.

Suggested change
&& typeof response === 'object'
&& typeof response === 'object'
&& 'response' in response

Copilot uses AI. Check for mistakes.
@mooreds mooreds requested a review from brob April 21, 2026 16:52
johnjeffers and others added 18 commits July 8, 2026 12:45
## [1.8.3-next-setup.1](v1.8.2...v1.8.3-next-setup.1) (2026-07-08)

### Bug Fixes

* **release:** adds back github token ([cd34c52](cd34c52))
* **release:** adds defaults ([fdac965](fdac965))
* **release:** adds permissions to action ([d66921b](d66921b))
* **release:** fixes commitlint issues ([ac51e58](ac51e58))
* **release:** makes release config common js ([ccefcad](ccefcad))
* **release:** removes PR action and corrects branch for testing push ([5a1059a](5a1059a))
The @semantic-release/git plugin was force-adding dist/** (via its
assets list) into every release commit, so compiled output was
committed to the repo and showed up in PR #45 — even though dist is
gitignored. git add --force overrides .gitignore, which is why it
landed despite the ignore rule.

- Remove 'dist/**' from the git plugin assets in release.config.cjs
- Untrack the already-committed dist/ (git rm -r --cached dist);
  it stays on disk, gitignored, and is rebuilt in CI before publish
- Remove unused deps: @comandeer/cli-spinner and command-exists
  (never imported), plus the redundant commitlint meta-package
  (kept @commitlint/cli, which provides the same binary)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
## [1.8.3-next-setup.2](v1.8.3-next-setup.1...v1.8.3-next-setup.2) (2026-07-08)

### Bug Fixes

* **release:** stop committing build output and drop unused deps ([d64ac2c](d64ac2c)), closes [#45](#45)
## [1.9.1-next-setup.1](v1.9.0...v1.9.1-next-setup.1) (2026-07-09)

### Bug Fixes

* **release:** adds back github token ([cd34c52](cd34c52))
* **release:** adds defaults ([fdac965](fdac965))
* **release:** adds permissions to action ([d66921b](d66921b))
* **release:** brings testing job into release action ([94551bd](94551bd))
* **release:** fixes commitlint issues ([ac51e58](ac51e58))
* **release:** fixes typo in workflow ([53d1443](53d1443))
* **release:** makes release config common js ([ccefcad](ccefcad))
* **release:** removes extraneous test that was causing failure in build not local ([687bbd6](687bbd6))
* **release:** removes PR action and corrects branch for testing push ([5a1059a](5a1059a))
* **release:** replaces inline testing with test workflow chaining ([d5b77b7](d5b77b7))
* **release:** stop committing build output and drop unused deps ([d64ac2c](d64ac2c)), closes [#45](#45)
* **release:** updates branches for workflow for main and next ([725d29c](725d29c))
* **release:** updates workflow away from quotations as per the github docs ([c6dbd1a](c6dbd1a))
## [1.9.1-next-setup.2](v1.9.1-next-setup.1...v1.9.1-next-setup.2) (2026-07-09)

### Bug Fixes

* **release:** adds dependency of test job ([3cd41cf](3cd41cf))
@brob brob requested review from a team as code owners July 13, 2026 14:06
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.

5 participants