fix(oauth): allow http scheme for localhost and return nil on invalid host by hardikkaurani · Pull Request #187 · microsoft/kiota-ruby

Hardik K (hardikkaurani) · 2026-10-21T05:31:03Z

Description

Fixes #117

This PR allows the http scheme for localhost and loopback targets in OAuthAccessTokenProvider, aligning Ruby with the behavior in Kiota Go (kiota-authentication-azure-go#102), Python, and PHP:

Allow HTTP on Localhost:
- Permits http scheme when the target host matches localhost, 127.0.0.1, ::1, or [::1].
- Preserves HTTPS requirement for all external hosts (raise StandardError, 'Only https is supported').
Proper Guard Return:
- Fixed missing return on return nil if !uri || !@host_validator.url_host_valid?(uri) which previously evaluated nil as a statement without returning, allowing URI(uri) to be called with invalid/nil targets.

Related Issue

Fixes #117

Testing

Added RSpec tests in components/authentication/oauth/spec/microsoft_kiota_authentication_oauth_spec.rb:

Verifies get_authorization_token returns nil for nil or disallowed hosts.
Verifies http on localhost, 127.0.0.1, and [::1] successfully creates and resumes authentication fiber.
Verifies http on external non-localhost hosts raises StandardError, 'Only https is supported'.

cc Vincent Biret (@baywet)

… host

Hardik K (hardikkaurani) · 2026-10-21T05:31:11Z

Hi Vincent Biret (@baywet), I've opened this PR to allow HTTP on localhost and loopback addresses in OAuthAccessTokenProvider (#117) and fix the guard return check for invalid hosts. Comprehensive RSpec tests are included. Ready for review!

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot

🟡 Changes recommended

The nil return breaks the authentication caller, explicit allowlists block local HTTP, and localhost bypasses all scheme restrictions.

3 open issues

_{Review effort: Balanced}

… localhost, and preserve fiber contract

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Hardik K (hardikkaurani) · 2026-10-21T14:34:32Z

Hi Vincent Biret (@baywet),

I've addressed the review feedback in the latest commit.

Changes

Host and scheme validation: Updated AllowedHostsValidator#url_host_valid? to validate hosts independently of the URL scheme, including bracketed and unbracketed IPv6 loopback addresses.
Strict scheme enforcement: Updated OAuthAccessTokenProvider#get_authorization_token to allow HTTPS for external hosts and HTTP only for localhost/loopback targets. Unsupported schemes are rejected.
Fiber contract and defensive handling: Preserved the Fiber-based return contract for invalid or disallowed URLs and added a nil check before resuming the token fiber in BaseBearerTokenAuthenticationProvider#authenticate_request.
Regression coverage: Added tests for scheme-independent host validation, nil token fibers, invalid and untrusted URLs, localhost/loopback authentication, unsupported schemes, external HTTP rejection, and end-to-end authentication flows.

The changes are ready for review. When you have a chance, could you please take another look? Thank you for your time and feedback!

Copilot

🟢 Approval recommended

Static review identified no unresolved blocking issues, and regression tests cover the corrected behavior; tests were not executed.

1 open issue

Allowlisted HTTP localhost URLs are rejected before host matching

2 resolved since last review

_{Review effort: Balanced}

Vincent Biret (baywet) · 2026-10-21T16:25:34Z

Hardik K (@hardikkaurani) the linting is failing, would you mind fixing this please?

Hardik K (hardikkaurani) · 2026-10-21T16:26:56Z

Vincent Biret (@baywet) Just doing it !

…op BlockLength

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Hardik K (hardikkaurani) · 2026-10-21T16:29:50Z

Hi Vincent Biret (@baywet)

I have updated the PR and resolved the RuboCop \Metrics/BlockLength\ offenses by modularizing the test suite into dedicated spec files matching the existing project structure (\�ase_bearer_token_authentication_provider_spec.rb\ and \oauth_access_token_provider_spec.rb).

All spec files are now well within RuboCop limits (< 152 lines).
All PR status checks (\Changelog PR Guard, \PullRequestConflicting, \license/cla) are green and the branch is up to date and mergeable.

Ready for review and workflow approval! Thank you!

Copilot

🟢 Approval recommended

Static review found no unresolved blocking issues, and targeted regression coverage addresses prior feedback; tests were not executed.

1 open issue

Allowlisted HTTP localhost URLs are rejected before host matching

_{Review effort: Balanced}

Hardik K (hardikkaurani) · 2026-10-21T16:49:50Z

Now also failing, just give me a minute !

I am taking a close look into this Vincent Biret (@baywet)

…boCop

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Hardik K (hardikkaurani) · 2026-10-21T17:14:58Z

Hi Vincent Biret (@baywet),

I have fixed the linting offense in components/authentication/oauth/spec/microsoft_kiota_authentication_oauth_spec.rb by removing the extra blank line before the block end (Layout/EmptyLinesAroundBlockBody), which was the single offense causing the lint and check-ruby-version-matrix jobs to fail.

All modified files have been verified to have zero RuboCop offenses. The fix has been pushed in commit 2732a7a and is ready for workflow approval and review!

… host validation on localhost in provider

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Hardik K (hardikkaurani) · 2026-10-21T17:39:42Z

Hi Vincent Biret (@baywet),

I have updated the PR in commit d51b090 following your review feedback:

Restored AllowedHostsValidator:
- Reverted components/abstractions/lib/microsoft_kiota_abstractions/authentication/allowed_hosts_validator.rb back to its original state from main, preserving return false unless parsed_url.is_a?(URI::HTTPS) and removing square bracket stripping.
Updated AllowedHostsValidator Spec:
- Applied your suggestion in components/abstractions/spec/request_information_uri_spec.rb to assert expect(validator.url_host_valid?('http://example.com')).to be(false).
Skipped Host Validation on Localhost Path in Provider:
- In OAuthAccessTokenProvider#get_authorization_token, host validation is now bypassed specifically when the target is localhost (return Fiber.new { nil } if !is_localhost && !@host_validator.url_host_valid?(uri)), while strictly enforcing that only http or https schemes are permitted on localhost.
Code Quality & RuboCop:
- Verified 0 offenses across all files.

Ready for your review!

Vincent Biret (baywet) · 2026-10-21T18:08:35Z

+  it 'validates host with https scheme and rejects non-https' do
+    validator = described_class.new(['example.com', 'localhost'])
+    expect(validator.url_host_valid?('http://example.com')).to be(false)
+    expect(validator.url_host_valid?('http://localhost:8080')).to be(false)


Suggested change

expect(validator.url_host_valid?('http://localhost:8080')).to be(false)

expect(validator.url_host_valid?('http://localhost:8080')).to be(true)

Updated in 6c131f5: Updated AllowedHostsValidator#url_host_valid? to allow http scheme for localhost / loopback targets when included in �llowed_hosts, and updated this test expectation to o be(true).

Copilot

🟡 Changes recommended

Loopback URLs bypass explicit host restrictions, and external HTTP handling contradicts the documented exception behavior.

3 open issues

_{Review effort: Balanced}

…idation from scheme enforcement

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Hardik K (hardikkaurani) · 2026-10-21T19:20:01Z

Hi Vincent Biret (@baywet),

I have updated the PR with commit 6c131f5 addressing your review suggestion and Copilot's review feedback:

Localhost & Loopback Support in AllowedHostsValidator:
- Updated AllowedHostsValidator#url_host_valid? to allow the http scheme for localhost, 127.0.0.1, ::1, and [::1] when included in allowed_hosts, while continuing to reject non-HTTPS external targets.
- Applied your suggested change in components/abstractions/spec/request_information_uri_spec.rb: expect(validator.url_host_valid?('http://localhost:8080')).to be(true).
Enforce Host Allowlist on Loopback Targets in OAuthAccessTokenProvider:
- Removed the loopback allowlist bypass in OAuthAccessTokenProvider#get_authorization_token. Host membership is now enforced against @host_validator for all targets independently of scheme, ensuring unapproved loopback targets return a fiber resolving to nil.
- Only explicitly allowlisted (or default empty) loopback targets receive tokens.
Separate Host Matching from Scheme Enforcement:
- Allowlisted external HTTP targets now cleanly reach the scheme validation step and raise StandardError, 'Only https is supported', while unapproved hosts silently return a fiber resolving to nil.
- Updated unit tests in oauth_access_token_provider_spec.rb to cover all combinations: allowlisted external HTTP target raising Only https is supported, unapproved loopbacks/externals returning nil, and allowlisted loopbacks obtaining tokens.

Ready for your review! Thank you!

Copilot

🟡 Changes recommended

IPv6 normalization rejects previously supported bracketed allowlist entries, leaving explicitly approved requests unauthenticated.

1 open issue

Preserve IPv6 brackets when validating allowed hosts · New

3 resolved since last review

_{Review effort: Balanced}

…IPv6 allowlist entries

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Hardik K (hardikkaurani) · 2026-10-23T13:28:23Z

Hi Vincent Biret (@baywet),

I have updated the PR with commit 8af8838 addressing the IPv6 allowlist matching feedback:

IPv6 Bracketed & Unbracketed Host Matching:
- In AllowedHostsValidator#url_host_valid?, matching now inspects parsed_url.host.downcase first before stripping brackets or falling back to parsed_url.hostname. This ensures bracketed allowlist entries like ['[::1]'] correctly match bracketed IPv6 URLs (https://[::1]:8080/v1.0 and http://[::1]:8080/v1.0) without being dropped by URI#hostname.
- Preserved stripped_host and hostname fallbacks to ensure unbracketed allowlist entries (['::1']) also match.
- Updated localhost? in both AllowedHostsValidator and OAuthAccessTokenProvider to check host first, followed by hostname.
Regression Test Coverage:
- Added specs in components/abstractions/spec/request_information_uri_spec.rb verifying both ['[::1]'] and ['::1'] allowlists over HTTP and HTTPS loopback URLs, as well as confirming external targets remain rejected.
- Updated components/authentication/oauth/spec/oauth_access_token_provider_spec.rb to validate end-to-end token acquisition for both bracketed and unbracketed allowlist entries.
Status:
- All tests across components/abstractions (127 examples) and components/authentication/oauth (11 examples) pass locally with 0 failures.
- PR checks (check-changelog, label-merge-conflicts, license/cla) are green and the branch is clean.

Ready for your review! Thank you!

Copilot

🔵 Needs a closer look

Authentication scheme and allowlist changes still need validation on a supported Ruby runtime.

No open issues 🎉

1 resolved since last review

Preserve IPv6 brackets when validating allowed hosts

_{Review effort: Balanced}

Hardik K (hardikkaurani) · 2026-10-23T14:15:02Z

Hi Vincent Biret (@baywet), I've addressed the review feedback and added regression tests for IPv6 allowlist handling. The relevant tests pass locally, and I've pushed the updates to the PR. When you have a chance, could you please take another look? Thank you for your time and feedback!

I've enjoyed contributing to Microsoft's open-source ecosystem and read your blog here: https://share.google/njEoOo6gDbfbit5Lf. I'd love to connect with you on LinkedIn and learn from your open-source experience. Thank you!

This PR is actually good to go with.

Hardik K (hardikkaurani) added 2 commits October 21, 2026 11:00

fix(oauth): allow http scheme for localhost and return nil on invalid…

adc38da

… host

test(oauth): add coverage for localhost http and host validation

47cc58f

Hardik K (hardikkaurani) requested a review from a team as a code owner October 21, 2026 05:31

Copilot AI balanced review requested due to automatic review settings October 21, 2026 05:31

Copilot AI reviewed Oct 21, 2026

Vincent Biret (baywet) requested a balanced review from Copilot October 21, 2026 12:17

Copilot started reviewing on behalf of Vincent Biret (baywet) October 21, 2026 12:18

View session

Copilot AI reviewed Oct 21, 2026

View reviewed changes

fix(oauth): separate host validation, strictly enforce http scheme on…

3ce36fd

… localhost, and preserve fiber contract

Copilot AI balanced review requested due to automatic review settings October 21, 2026 14:32

Copilot AI reviewed Oct 21, 2026

Vincent Biret (baywet) requested a balanced review from Copilot October 21, 2026 16:10

Copilot started reviewing on behalf of Vincent Biret (baywet) October 21, 2026 16:11

View session

Copilot AI reviewed Oct 21, 2026

View reviewed changes

test(specs): split authentication provider specs to comply with RuboC…

ffb6526

…op BlockLength

Copilot AI balanced review requested due to automatic review settings October 21, 2026 16:29

Copilot AI reviewed Oct 21, 2026

Vincent Biret (baywet) requested a balanced review from Copilot October 21, 2026 16:40

Copilot started reviewing on behalf of Vincent Biret (baywet) October 21, 2026 16:40

View session

Copilot AI reviewed Oct 21, 2026

View reviewed changes

Vincent Biret (baywet) requested changes Oct 21, 2026

View reviewed changes

style(oauth): remove extra empty line at block body end to satisfy Ru…

2732a7a

…boCop

Copilot AI balanced review requested due to automatic review settings October 21, 2026 17:14

Copilot AI reviewed Oct 21, 2026

fix(oauth): preserve AllowedHostsValidator https enforcement and skip…

d51b090

… host validation on localhost in provider

Copilot AI balanced review requested due to automatic review settings October 21, 2026 17:38

Copilot AI reviewed Oct 21, 2026

Hardik K (hardikkaurani) requested a review from Vincent Biret (baywet) October 21, 2026 17:47

Vincent Biret (baywet) requested a balanced review from Copilot October 21, 2026 18:08

Vincent Biret (baywet) reviewed Oct 21, 2026

View reviewed changes

Copilot started reviewing on behalf of Vincent Biret (baywet) October 21, 2026 18:11

View session

Copilot AI reviewed Oct 21, 2026

View reviewed changes

Comment thread ...authentication/oauth/lib/microsoft_kiota_authentication_oauth/oauth_access_token_provider.rb Outdated

Comment thread components/authentication/oauth/spec/oauth_access_token_provider_spec.rb Outdated

fix(oauth): enforce host allowlist on loopbacks and separate host val…

6c131f5

…idation from scheme enforcement

Copilot AI balanced review requested due to automatic review settings October 21, 2026 19:19

Copilot AI reviewed Oct 21, 2026

Hardik K (hardikkaurani) requested a review from Vincent Biret (baywet) October 21, 2026 19:21

Vincent Biret (baywet) mentioned this pull request Oct 23, 2026

fix(oauth): derive default scopes per request #190

Open

Vincent Biret (baywet) requested a balanced review from Copilot October 23, 2026 12:17

Vincent Biret (baywet) mentioned this pull request Oct 23, 2026

Missing refresh token when using client credentials #116

Open

Copilot started reviewing on behalf of Vincent Biret (baywet) October 23, 2026 12:18

View session

Copilot AI reviewed Oct 23, 2026

View reviewed changes

Comment thread ...ents/abstractions/lib/microsoft_kiota_abstractions/authentication/allowed_hosts_validator.rb Outdated

fix(abstractions): match parsed_url.host first to preserve bracketed …

8af8838

…IPv6 allowlist entries

Copilot AI balanced review requested due to automatic review settings October 23, 2026 13:26

Copilot AI reviewed Oct 23, 2026

Vincent Biret (baywet) requested a balanced review from Copilot October 23, 2026 13:48

Copilot started reviewing on behalf of Vincent Biret (baywet) October 23, 2026 13:48

View session

Copilot AI reviewed Oct 23, 2026

View reviewed changes

	expect(validator.url_host_valid?('http://localhost:8080')).to be(false)
	expect(validator.url_host_valid?('http://localhost:8080')).to be(true)

Conversation

Hardik K (hardikkaurani) commented Oct 21, 2026

Description

Related Issue

Testing

Uh oh!

Hardik K (hardikkaurani) commented Oct 21, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Hardik K (hardikkaurani) commented Oct 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

Vincent Biret (baywet) commented Oct 21, 2026

Uh oh!

Hardik K (hardikkaurani) commented Oct 21, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Hardik K (hardikkaurani) commented Oct 21, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

Hardik K (hardikkaurani) commented Oct 21, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Hardik K (hardikkaurani) commented Oct 21, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Hardik K (hardikkaurani) commented Oct 21, 2026

Uh oh!

Vincent Biret (baywet) Oct 21, 2026

Choose a reason for hiding this comment

Uh oh!

Hardik K (hardikkaurani) Oct 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Hardik K (hardikkaurani) commented Oct 21, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Hardik K (hardikkaurani) commented Oct 21, 2026 •

edited

Loading

Hardik K (hardikkaurani) commented Oct 23, 2026 •

edited

Loading