Skip to content

brokers: Rename setting to force_access_check_with_provider#1444

Merged
adombeck merged 1 commit intomainfrom
1443-rename-force-provider-access-check
Apr 15, 2026
Merged

brokers: Rename setting to force_access_check_with_provider#1444
adombeck merged 1 commit intomainfrom
1443-rename-force-provider-access-check

Conversation

@adombeck
Copy link
Copy Markdown
Contributor

@adombeck adombeck commented Apr 14, 2026

We've had reports that users expect the force_provider_authentication setting to always require device authentication during login. That's not the case, it instead forces a token refresh during login, which fails if the user does not have the necessary permissions in the identity provider.

The new name should hopefully make the setting clearer.

A few considerations:

  • We chose "force" instead of "require" or "always" to signal risk, because it's a major UX issue if users are not able to log in when they have network connectivity issues.

  • For the same reason, we decided to keep the setting opt-in.

  • We chose force_access_check_with_provider over the shorter force_provider_access_check for clarity and closeness to natural language.

Closes #1443

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 14, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.28%. Comparing base (15c4664) to head (9d068d2).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1444      +/-   ##
==========================================
- Coverage   87.05%   80.28%   -6.78%     
==========================================
  Files          93       20      -73     
  Lines        6367      999    -5368     
  Branches      111        0     -111     
==========================================
- Hits         5543      802    -4741     
+ Misses        768      197     -571     
+ Partials       56        0      -56     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@nooreldeenmansour nooreldeenmansour linked an issue Apr 14, 2026 that may be closed by this pull request
Copy link
Copy Markdown
Member

@nooreldeenmansour nooreldeenmansour left a comment

Choose a reason for hiding this comment

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

The new description is less technical and more functionality-focused. I don't think that'll be an issue for most users, if anything it should be less confusing. so I agree with the approach and the new name:)

Edit:
P.S., I linked #1443, if you are addressing #1340 as well in the same PR, consider linking it as well, so we don't forget about it

@adombeck adombeck marked this pull request as ready for review April 14, 2026 22:04
We've had reports that users expect the force_provider_authentication
setting to always require device authentication during login. That's not
the case, it instead forces a token refresh during login, which fails if
the user does not have the necessary permissions in the identity
provider.

The new name should hopefully make the setting clearer.

A few considerations:

* We chose "force" instead of "require" or "always" to signal risk,
  because it's a major UX issue if users are not able to log in when
  they have network connectivity issues.

* For the same reason, we decided to keep the setting opt-in.

* We chose force_access_check_with_provider over the shorter
  force_provider_access_check for clarity and closeness to natural
  language.
@adombeck adombeck force-pushed the 1443-rename-force-provider-access-check branch from 9d068d2 to de17fff Compare April 15, 2026 10:57
@adombeck adombeck changed the title brokers: Rename setting to force_provider_access_check brokers: Rename setting to force_access_check_with_provider Apr 15, 2026
@adombeck adombeck requested a review from edibotopic April 15, 2026 10:58
Copy link
Copy Markdown
Contributor

@edibotopic edibotopic left a comment

Choose a reason for hiding this comment

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

Good change. Thanks for updating the docs too.

@adombeck adombeck merged commit 82cb80b into main Apr 15, 2026
23 of 29 checks passed
@adombeck adombeck deleted the 1443-rename-force-provider-access-check branch April 15, 2026 12:10
@adombeck
Copy link
Copy Markdown
Contributor Author

Thanks for updating the docs too.

@edibotopic note that I only updated the setting name. we might still want to the rest of the section. I'll leave #1340 open for that.

@edibotopic
Copy link
Copy Markdown
Contributor

Thanks for updating the docs too.

@edibotopic note that I only updated the setting name. we might still want to the rest of the section. I'll leave #1340 open for that.

Yes, that makes sense @adombeck .

@nooreldeenmansour nooreldeenmansour restored the 1443-rename-force-provider-access-check branch April 17, 2026 11:22
@nooreldeenmansour nooreldeenmansour deleted the 1443-rename-force-provider-access-check branch April 17, 2026 11:25
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.

Consider renaming force_provider_authentication

4 participants