Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

SecuritySettingsSource license.self_generated=trial #38233

Merged
merged 1 commit into from
Feb 5, 2019
Merged

SecuritySettingsSource license.self_generated=trial #38233

merged 1 commit into from
Feb 5, 2019

Conversation

albertzaharovits
Copy link
Contributor

Authn is enabled only if license_type is non basic, but basic is what the LicenseService generates implicitly. This commit explicitly sets license type to trial, which allows for authn, in the SecuritySettingsSource which is the settings configuration parameter for InternalTestClusters.

The real problem, that had created tests failures like #31028 and #32685, is that the check licenseState.isAuthAllowed() can change sporadically, especially because the status field of the licenseState object is not volatile. If it were to return true or false during the whole test there would be no problem. The problem manifests when it turns from true to false right before Realms.asList(). There are other license checks before this one (request filter, token service, etc) that would not cause a problem if they would suddenly see the check as false. But switching to false before Realms.asList() makes it appear that no installed realms could have handled the authn token which is an authentication error, as can be seen in the failing tests.

The tip off was that the stacktraces indicated that the IteratingActionListener reached the end of the realm list without success.

Closes #31028 #32685

@albertzaharovits albertzaharovits added >test Issues or PRs that are addressing/adding tests v7.0.0 :Security/Security Security issues without another label v6.7.0 v6.6.1 labels Feb 1, 2019
@albertzaharovits albertzaharovits self-assigned this Feb 1, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-security

tvernum
tvernum approved these changes Feb 4, 2019
View reviewed changes
Copy link
Contributor

@tvernum tvernum left a comment

Choose a reason for hiding this comment

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

LGTM

jaymode
jaymode approved these changes Feb 4, 2019
View reviewed changes
Copy link
Member

@jaymode jaymode left a comment

Choose a reason for hiding this comment

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

LGTM.

An aside, I do not think status not being volatile has anything to do with the issue; all methods are synchronized so we do not need to declare status as volatile

@albertzaharovits
Copy link
Contributor Author

An aside, I do not think status not being volatile has anything to do with the issue; all methods are synchronized so we do not need to declare status as volatile

This is correct, my bad!

@albertzaharovits albertzaharovits merged commit 8e2eb39 into elastic:master Feb 5, 2019
@albertzaharovits albertzaharovits deleted the integ_test_self_generated branch February 5, 2019 08:49
albertzaharovits added a commit to albertzaharovits/elasticsearch that referenced this pull request Feb 5, 2019
@albertzaharovits
SecuritySettingsSource license.self_generated: trial (elastic#38233)
cc3645d 
Authn is enabled only if `license_type` is non `basic`, but `basic` is
what the `LicenseService` generates implicitly. This commit explicitly sets
license type to `trial`, which allows for authn, in the `SecuritySettingsSource`
which is the settings configuration parameter for `InternalTestCluster`s.

The real problem, that had created tests failures like elastic#31028 and elastic#32685, is
that the check `licenseState.isAuthAllowed()` can change sporadically. If it were
to return `true` or `false` during the whole test there would be no problem.
The problem manifests when it turns from `true` to `false` right before `Realms.asList()`.
There are other license checks before this one (request filter, token service, etc)
that would not cause a problem if they would suddenly see the check as `false`.
But switching to `false` before `Realms.asList()` makes it appear that no installed
realms could have handled the authn token which is an authentication error, as can
be seen in the failing tests.

Closes elastic#31028 elastic#32685
albertzaharovits added a commit to albertzaharovits/elasticsearch that referenced this pull request Feb 5, 2019
@albertzaharovits
SecuritySettingsSource license.self_generated: trial (elastic#38233)
bc0270d 
Authn is enabled only if `license_type` is non `basic`, but `basic` is
what the `LicenseService` generates implicitly. This commit explicitly sets
license type to `trial`, which allows for authn, in the `SecuritySettingsSource`
which is the settings configuration parameter for `InternalTestCluster`s.

The real problem, that had created tests failures like elastic#31028 and elastic#32685, is
that the check `licenseState.isAuthAllowed()` can change sporadically. If it were
to return `true` or `false` during the whole test there would be no problem.
The problem manifests when it turns from `true` to `false` right before `Realms.asList()`.
There are other license checks before this one (request filter, token service, etc)
that would not cause a problem if they would suddenly see the check as `false`.
But switching to `false` before `Realms.asList()` makes it appear that no installed
realms could have handled the authn token which is an authentication error, as can
be seen in the failing tests.

Closes elastic#31028 elastic#32685
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Feb 5, 2019
@jasontedor
Merge branch 'master' into simplify-retention-lease-expiration
ff9520e 
* master:
  Mute failing API key integration test (elastic#38409)
  Change the milliseconds precision to 3 digits for intervals. (elastic#38297)
  SecuritySettingsSource license.self_generated: trial (elastic#38233)
  Rename no-master-block setting (elastic#38350)
  Rename static Zen1 settings (elastic#38333)
  Migration doc for audit json log file (elastic#38165)
  Add apm_user reserved role (elastic#38206)
albertzaharovits added a commit that referenced this pull request Feb 5, 2019
@albertzaharovits
SecuritySettingsSource license.self_generated: trial (#38233) (#38397)
a123582 
Authn is enabled only if `license_type` is non `basic`, but `basic` is
what the `LicenseService` generates implicitly. This commit explicitly sets
license type to `trial`, which allows for authn, in the `SecuritySettingsSource`
which is the settings configuration parameter for `InternalTestCluster`s.

The real problem, that had created tests failures like #31028 and #32685, is
that the check `licenseState.isAuthAllowed()` can change sporadically. If it were
to return `true` or `false` during the whole test there would be no problem.
The problem manifests when it turns from `true` to `false` right before `Realms.asList()`.
There are other license checks before this one (request filter, token service, etc)
that would not cause a problem if they would suddenly see the check as `false`.
But switching to `false` before `Realms.asList()` makes it appear that no installed
realms could have handled the authn token which is an authentication error, as can
be seen in the failing tests.

Closes #31028 #32685
albertzaharovits added a commit that referenced this pull request Feb 5, 2019
@albertzaharovits
SecuritySettingsSource license.self_generated: trial (#38233) (#38398)
3131f59 
Authn is enabled only if `license_type` is non `basic`, but `basic` is
what the `LicenseService` generates implicitly. This commit explicitly sets
license type to `trial`, which allows for authn, in the `SecuritySettingsSource`
which is the settings configuration parameter for `InternalTestCluster`s.

The real problem, that had created tests failures like #31028 and #32685, is
that the check `licenseState.isAuthAllowed()` can change sporadically. If it were
to return `true` or `false` during the whole test there would be no problem.
The problem manifests when it turns from `true` to `false` right before `Realms.asList()`.
There are other license checks before this one (request filter, token service, etc)
that would not cause a problem if they would suddenly see the check as `false`.
But switching to `false` before `Realms.asList()` makes it appear that no installed
realms could have handled the authn token which is an authentication error, as can
be seen in the failing tests.

Closes #31028 #32685
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Feb 8, 2019
@jasontedor
Merge branch '6.6' into docker-from-artifacts-6.6
8c938a9 
* 6.6: (121 commits)
  [DOCS] Add warning about bypassing ML PUT APIs (elastic#38608)
  fix dissect doc "ip" --> "clientip" (elastic#38512)
  bad formatted JSON object (elastic#38515)
  SQL: Fix issue with IN not resolving to underlying keyword field (elastic#38440)
  Update ilm-api.asciidoc, point to REMOVE policy (elastic#38235)
  Backport changes to the release notes script. (elastic#38347)
  Change the milliseconds precision to 3 digits for intervals. (elastic#38297)
  SecuritySettingsSource license.self_generated: trial (elastic#38233) (elastic#38398)
  Fix IndexAuditTrail rolling upgrade on rollover edge 2 (elastic#38286) (elastic#38381)
  Cleanup construction of interceptors (elastic#38388)
  Skip unsupported languages for tests (elastic#38328) (elastic#38385)
  [ILM][TEST] increase assertBusy timeout (elastic#36864) (elastic#38354)
  Docs: Drop inline callout from scroll example (elastic#38340) (elastic#38365)
  Preserve ILM operation mode when creating new lifecycles (elastic#38134) (elastic#38230)
  [ML] Add explanation so far to file structure finder exceptions (elastic#38337)
  ML: Fix error race condition on stop _all datafeeds and close _all jobs (elastic#38113) (elastic#38211) (elastic#38222)
  SQL: Generate relevant error message when grouping functions are not used in GROUP BY (elastic#38017)
  Fix NPE in Logfile Audit Filter (elastic#38120) (elastic#38273)
  Enable trace log in FollowerFailOverIT (elastic#38148)
  Replace awaitBusy with assertBusy in atLeastDocsIndexed (elastic#38190)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tvernum tvernum tvernum approved these changes

@jaymode jaymode jaymode approved these changes

Assignees

@albertzaharovits albertzaharovits

Labels
:Security/Security Security issues without another label >test Issues or PRs that are addressing/adding tests v6.6.1 v6.7.0 v7.0.0-beta1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@albertzaharovits @elasticmachine @tvernum @jaymode @colings86