[PkiRealm] Invalidate cache on role mappings change#31510
Merged
bizybot merged 4 commits intoelastic:masterfrom Jun 22, 2018
Merged
[PkiRealm] Invalidate cache on role mappings change#31510bizybot merged 4 commits intoelastic:masterfrom
bizybot merged 4 commits intoelastic:masterfrom
Conversation
PkiRealm caches successful authentications and provides ways to invalidate cache. But in some scenario's the cache was not being invalidated on role mapping change. PkiRealm does not inform role mapper to be notified for cache refresh on role mapping updates. The logic in `TransportClearRealmCacheAction#nodeOperation` which gets invoked for refreshing cache on realms, considers null or empty realm names in request as clear cache on all realms. When ldap realm is not present then it clears cache for all realms so it works fine, but when ldap realm is configured then role mapper sends request with ldap realm names and so cache is cleared only for those realms. This commit resolves the issue by registering PkiRealm with role mapper for cache refresh. PkiRealm implements CachingRealm and as it does not extend CachingUsernamePasswordRealm, have modified the interface method `refreshRealmOnChange` to accept CachingRealm.
Collaborator
|
Pinging @elastic/es-security |
jaymode
approved these changes
Jun 21, 2018
Member
jaymode
left a comment
There was a problem hiding this comment.
Left a minor comment, otherwise LGTM
| UserRoleMapper roleMapper = mock(UserRoleMapper.class); | ||
| PkiRealm realm = new PkiRealm(new RealmConfig("", Settings.EMPTY, globalSettings, TestEnvironment.newEnvironment(globalSettings), | ||
| new ThreadContext(globalSettings)), roleMapper); | ||
| verify(roleMapper).refreshRealmOnChange(realm); |
Member
There was a problem hiding this comment.
how about adding a verifyNoMoreInteractions(roleMapper) at the end of this test
dnhatn
added a commit
that referenced
this pull request
Jun 23, 2018
* master: Add get field mappings to High Level REST API Client (#31423) [DOCS] Updates Watcher examples for code testing (#31152) TEST: Add bwc recovery tests with synced-flush index [DOCS] Move sql to docs (#31474) [DOCS] Move monitoring to docs folder (#31477) Core: Combine doExecute methods in TransportAction (#31517) IndexShard should not return null stats (#31528) fix repository update with the same settings but different type (#31458) Fix Mockito trying to mock IOException that isn't thrown by method (#31433) (#31527) Node selector per client rather than per request (#31471) Core: Combine messageRecieved methods in TransportRequestHandler (#31519) Upgrade to Lucene 7.4.0. (#31529) [ML] Add ML filter update API (#31437) Allow multiple unicast host providers (#31509) Avoid deprecation warning when running the ML datafeed extractor. (#31463) REST high-level client: add simulate pipeline API (#31158) Get Mapping API to honour allow_no_indices and ignore_unavailable (#31507) [PkiRealm] Invalidate cache on role mappings change (#31510) [Security] Check auth scheme case insensitively (#31490) In NumberFieldType equals and hashCode, make sure that NumberType is taken into account. (#31514) [DOCS] Fix REST tests in SQL docs [DOCS] Add code snippet testing in more ML APIs (#31339) Core: Remove ThreadPool from base TransportAction (#31492) [DOCS] Remove fixed file from build.gradle Rename createNewTranslog to fileBasedRecovery (#31508) Test: Skip assertion on windows [DOCS] Creates field and document level security overview (#30937) [DOCS] Significantly improve SQL docs [DOCS] Move migration APIs to docs (#31473) Core: Convert TransportAction.execute uses to client calls (#31487) Return transport addresses from UnicastHostsProvider (#31426) Ensure local addresses aren't null (#31440) Remove unused generic type for client execute method (#31444) Introduce http and tcp server channels (#31446)
bizybot
added a commit
that referenced
this pull request
Jul 9, 2018
PkiRealm caches successful authentications and provides ways to invalidate the cache. But in some scenario's the cache was not being invalidated on role mapping change. PkiRealm does not inform role mapper to be notified for cache refresh on role mapping updates. The logic in `TransportClearRealmCacheAction#nodeOperation` which gets invoked for refreshing cache on realms, considers null or empty realm names in the request as clear cache on all realms. When LDAP realm is not present then it clears cache for all realms so it works fine, but when LDAP realm is configured then role mapper sends a request with LDAP realm names and so the cache is cleared only for those realms. This commit resolves the issue by registering PkiRealm with role mapper for cache refresh. PkiRealm implements CachingRealm and as it does not extend CachingUsernamePasswordRealm, have modified the interface method `refreshRealmOnChange` to accept CachingRealm.
dnhatn
added a commit
that referenced
this pull request
Jul 12, 2018
* 6.x: Force execution of fetch tasks (#31974) [TEST] Mute SlackMessageTests.testTemplateRender Docs: Explain closing the high level client [test] disable java packaging tests for suse XContentTests : Insert random fields at random positions (#30867) Add Get Snapshots High Level REST API (#31980) Fix unreachable error condition in AmazonS3Fixture (#32005) [6.x][ML] Ensure immutability of MlMetadata (#31994) Add Expected Reciprocal Rank metric (#31891) SQL: Add support for single parameter text manipulating functions (#31874) muted tests due to #31940 Work around reported problem in eclipse (#31960) Move build integration tests out of :buildSrc project (#31961) [Test] Reactive 3rd party tests on CI (#31919) Fix assertIngestDocument wrongfully passing (#31913) (#31951) SQL: Support for escape sequences (#31884) SQL: HAVING clause should accept only aggregates (#31872) Docs: fix typo in datehistogram (#31972) Switch url repository rest tests to new style requests (#31944) Switch reindex tests to new style requests (#31941) Switch test framework to new style requests (#31939) Docs: Added note about cloud service to installation and getting started [DOCS] Removes alternative docker pull example (#31934) ingest: date_index_name processor template resolution (#31841) Test: fix null failure in watcher test (#31968) Watcher: Slack message empty text (#31596) Switch low level rest tests to new style Requests (#31938) Switch high level rest tests to new style requests (#31937) HLREST: Bundle the x-pack protocol project (#31904) [ML] Mute test failing due to Java 11 date time format parsing bug (#31899) Increase logging level for testStressMaybeFlush rolling upgrade should use a replica to prevent relocations while running a scroll [test] port archive distribution packaging tests (#31314) HLRest: Move xPackInfo() to xPack().info() (#31905) Increase logging level for :qa:rolling-upgrade Backport: Add template config for Beat state to X-Pack Monitoring (#31809) (#31893) Fix building AD URL from domain name (#31849) Fix broken NaN check in MovingFunctions#stdDev() (#31888) Change trappy float comparison (#31889) Add opaque_id to audit logging (#31878) add support for is_write_index in put-alias body parsing (#31674) Ingest: Enable Templated Fieldnames in Rename (#31690) (#31896) Ingest: Add ignore_missing option to RemoveProc (#31693) (#31892) [Docs] Fix typo in the Rollup API Quick Reference (#31855) Watcher: Add ssl.trust email account setting (#31684) [PkiRealm] Invalidate cache on role mappings change (#31510) [Security] Check auth scheme case insensitively (#31490) HLREST: Add x-pack-info API (#31870) Remove link to oss-MSI (#31844) Painless: Restructure Definition/Whitelist (#31879)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PkiRealm caches successful authentications and provides ways to
invalidate the cache. But in some scenario's the cache was not being
invalidated on role mapping change.
PkiRealm does not inform role mapper to get notified for cache
refresh on role mapping updates as it does not invoke
UserRoleMapper#refreshRealmOnChangeThe logic in
TransportClearRealmCacheAction#nodeOperationwhich getsinvoked for refreshing cache on realms, considers null or empty
realm names in the request as to clear cache on all realms. When LDAP realm
is not configured then it clears cache for all realms so it works fine in that case,
but when LDAP realm is configured then role mapper sends a request with
LDAP realm name(s) and so the cache is cleared only for those realms.
This commit resolves the issue by registering PkiRealm with role
mapper for cache refresh. PkiRealm implements CachingRealm and as it
does not extend CachingUsernamePasswordRealm, have modified the
interface method
refreshRealmOnChangeto accept CachingRealm.