Track notifications from the Auth service #560
Merged
Conversation
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
amrc-benmorrow
force-pushed
the
bmz/auth-rx-client
branch
4 times, most recently
from
91f04a0 to
0ca1421
Compare
Contributor
Author
|
Causes failures in cluster-manager on initial install. The failures are caused by subscriptions to the ConfigDB being refused. I thought this was to do with permissions being cached inappropriately in RxClient.Auth but this doesn't appear to be the cause. The failures go away once auth and configdb have been restarted. |
Contributor
Author
|
RESOLVED The cause here is that adding a new principal UPN mapping is not causing the permissions for that principal to be correctly recalculated internally. This problem lies entirely inside the Auth service. The ConfigDB is performing a WATCH on This can be reliably reproduced by:
|
amrc-benmorrow
force-pushed
the
bmz/auth-rx-client
branch
from
cc1fb6b to
7688799
Compare
This overrides the core acl-fetch method in the Auth interface with one which subscribes to notifications. This gives us immediate updates when permissions change. We subscribe to the ACL for a given principal for half an hour; if we have no more requests in that time the subscription will be dropped.
This gives push authentication.
Code which has moved from acs-configdb to js-service-api was not moved across correctly.
Skip the Directory for now as that needs direct access to the raw ACL.
The Directory needs access to the raw ACL, so expose this as a public method. It's cleaner to override a public method in any case.
This should be more reliable as the Sparkplug notifications do not sync well with the HTTP caching.
The ServiceClient already had correct code to return a 503 when we got permission denied from the Auth service. Use this instead of overriding it. Switch to Response in place of Maybe as this preserves the HTTP response code.
I even created a sequence function especially for this purpose...
Rationalise our error returns in ServiceClient so that we only return ServiceError for a problem with the remote service. Ordinary parameter errors now throw other errors. This means that a ServiceError thrown during processing of a request should always invoke a 503 response. Remove the hack in Auth.fetch_acl to throw a ServiceError with 503 in.
The original logic here did not retry failed updates and left this to the 10-minute reconcile loop. Changing it to retry individual actions causes a retrying update to block all other update, including an update to delete the cluster. Group actions by cluster. Attach retry logic to each action and then switch through the actions for each cluster. This ensures we abandon a retrying action if we should be doing something else instead.
Instead of duck-typing use a proper class to distinguish between 'a service we contacted returned an error', 'we need to return a specific error' and other failures which happen to result in an error object with a `status` property.
amrc-benmorrow
force-pushed
the
bmz/auth-rx-client
branch
from
7688799 to
d2a9f52
Compare
Move bootstrap ACL handling into this method; this now returns the full ACL, but cannot handle root rights within the format of the return value. In any case cmdesc must do its own root handling. Carry through the previous change here into the Directory code.
Instead of looking up addresses and CCL templates when we need them, and relying on the HTTP caching, just track the whole lot via notify. This simplifies a lot of the logic. Remove redundant MQTT logic concerned with being a Sparkplug Node. We don't need to do that any more.
amrc-benmorrow
force-pushed
the
bmz/auth-rx-client
branch
from
579c175 to
2016e4a
Compare
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.
Add an Auth interface to RxClient. This tracks ACLs via change-notify, caching and tracking the ACL for a given principal for half an hour. Calls to the existing Auth interface methods will now go via this cache instead of the HTTP request cache.
Update existing Auth clients to use RxClient. In most cases this is all that is required to support change-notify. In some cases additional updates to use the ConfigDB notify interface are also included. After this the only remaining use of the Sparkplug change-notify interface is in the Monitor; see #575.