ololo3000 commented on a change in pull request #8892:
URL: https://github.com/apache/ignite/pull/8892#discussion_r615022549
##########
File path:
modules/core/src/main/java/org/apache/ignite/internal/processors/odbc/ClientListenerAbstractConnectionContext.java
##########
@@ -104,33 +95,12 @@ public GridKernalContext kernalContext() {
/**
* Perform authentication.
*
- * @return Auth context.
* @throws IgniteCheckedException If failed.
*/
- protected AuthorizationContext authenticate(GridNioSession ses, String
user, String pwd)
- throws IgniteCheckedException {
- if (ctx.security().enabled())
- authCtx = authenticateExternal(ses, user,
pwd).authorizationContext();
- else if (ctx.authentication().enabled()) {
- if (F.isEmpty(user))
- throw new IgniteAccessControlException("Unauthenticated
sessions are prohibited.");
-
- authCtx = ctx.authentication().authenticate(user, pwd);
+ protected void authenticate(GridNioSession ses, String user, String pwd)
throws IgniteCheckedException {
Review comment:
We should not consider AuthProcessor as the only security
implementation. It can be replaced with the custom implementation by the user
and nothing restricts null to be returned. So the check is justified.
I have fixed java doc.
##########
File path:
modules/core/src/main/java/org/apache/ignite/internal/processors/authentication/IgniteAuthenticationProcessor.java
##########
@@ -971,6 +893,81 @@ private static boolean isNodeHoldsUsers(ClusterNode n) {
return !n.isClient() && !n.isDaemon();
}
+ /** {@inheritDoc} */
+ @Override public SecurityContext authenticateNode(ClusterNode node,
SecurityCredentials cred) throws IgniteCheckedException {
+ return new SecurityContextImpl(
+ node.id(),
+ node.attribute(ATTR_IGNITE_INSTANCE_NAME),
+ REMOTE_NODE,
+ new InetSocketAddress(F.first(node.addresses()), 0));
+ }
+
+ /** {@inheritDoc} */
+ @Override public SecuritySubject authenticatedSubject(UUID subjId) throws
IgniteCheckedException {
+ throw new UnsupportedOperationException();
+ }
+
+ /** {@inheritDoc} */
+ @Override public Collection<SecuritySubject> authenticatedSubjects()
throws IgniteCheckedException {
+ throw new UnsupportedOperationException();
+ }
+
+ /** {@inheritDoc} */
+ @Override public boolean isGlobalNodeAuthentication() {
+ return false;
+ }
+
+ /** {@inheritDoc} */
+ @Override public void authorize(String name, SecurityPermission perm,
SecurityContext securityCtx) throws SecurityException {
+ if (!userOps.contains(perm))
+ return;
+
+ SecuritySubject subj = ctx.security().securityContext().subject();
+
+ Object login = subj.login();
+
+ if (subj.type() == REMOTE_NODE) {
+ throw new SecurityException("User management operations initiated
on behalf of the Ignite node" +
+ " are not supported [igniteInstanceName=" + login + ']');
+ }
+
+ if (!DEFAULT_USER_NAME.equals(login) && !(ALTER_USER == perm &&
Objects.equals(login, name)))
+ throw new SecurityException("User management operations are not
allowed for user [curUser=" + login + ']');
+
+ if (DROP_USER == perm && DEFAULT_USER_NAME.equals(name))
+ throw new SecurityException("Default user cannot be removed.");
+ }
+
+ /** {@inheritDoc} */
+ @Override public void onSessionExpired(UUID subjId) {
+ // No-op.
+ }
+
+ /** {@inheritDoc} */
+ @Override public SecurityContext securityContext(UUID subjId) {
+ User user = users.get(subjId);
+
+ return user == null ? null : new SecurityContextImpl(subjId,
user.name(), REMOTE_CLIENT, null);
+ }
+
+ /**
+ * Gets the user with the specified ID and login. It is necessary to check
the login to make sure that there was
+ * no collision when calculating the user ID.
+ */
+ private User findUser(UUID subjId, String login) {
+ User user = users.get(subjId);
Review comment:
My opinion is - we should not use lock there.
1. We just read from user map that is already ConcurrentMap.
2. As I see the main purpose of this mux is to to keep user operations
consistent in case they should be send to a new joining node. But frankly it
does not always work as expected see
https://issues.apache.org/jira/browse/IGNITE-14301
##########
File path:
modules/core/src/main/java/org/apache/ignite/internal/processors/authentication/IgniteAuthenticationProcessor.java
##########
@@ -971,6 +893,81 @@ private static boolean isNodeHoldsUsers(ClusterNode n) {
return !n.isClient() && !n.isDaemon();
}
+ /** {@inheritDoc} */
+ @Override public SecurityContext authenticateNode(ClusterNode node,
SecurityCredentials cred) throws IgniteCheckedException {
+ return new SecurityContextImpl(
+ node.id(),
+ node.attribute(ATTR_IGNITE_INSTANCE_NAME),
+ REMOTE_NODE,
+ new InetSocketAddress(F.first(node.addresses()), 0));
+ }
+
+ /** {@inheritDoc} */
+ @Override public SecuritySubject authenticatedSubject(UUID subjId) throws
IgniteCheckedException {
+ throw new UnsupportedOperationException();
+ }
+
+ /** {@inheritDoc} */
+ @Override public Collection<SecuritySubject> authenticatedSubjects()
throws IgniteCheckedException {
+ throw new UnsupportedOperationException();
+ }
+
+ /** {@inheritDoc} */
+ @Override public boolean isGlobalNodeAuthentication() {
+ return false;
+ }
+
+ /** {@inheritDoc} */
+ @Override public void authorize(String name, SecurityPermission perm,
SecurityContext securityCtx) throws SecurityException {
+ if (!userOps.contains(perm))
+ return;
+
+ SecuritySubject subj = ctx.security().securityContext().subject();
+
+ Object login = subj.login();
+
+ if (subj.type() == REMOTE_NODE) {
+ throw new SecurityException("User management operations initiated
on behalf of the Ignite node" +
+ " are not supported [igniteInstanceName=" + login + ']');
+ }
+
+ if (!DEFAULT_USER_NAME.equals(login) && !(ALTER_USER == perm &&
Objects.equals(login, name)))
+ throw new SecurityException("User management operations are not
allowed for user [curUser=" + login + ']');
+
+ if (DROP_USER == perm && DEFAULT_USER_NAME.equals(name))
+ throw new SecurityException("Default user cannot be removed.");
+ }
+
+ /** {@inheritDoc} */
+ @Override public void onSessionExpired(UUID subjId) {
+ // No-op.
+ }
+
+ /** {@inheritDoc} */
+ @Override public SecurityContext securityContext(UUID subjId) {
+ User user = users.get(subjId);
+
+ return user == null ? null : new SecurityContextImpl(subjId,
user.name(), REMOTE_CLIENT, null);
Review comment:
SecurityContext associated with the node is stored in node attributes
and is obtained automatically by Ignite using the node ID (see
IgniteSecurityProcessor#withContext(java.util.UUID)). As we use node ID as
subject ID during node authentication, this method makes sense only for the
REMOTE_CLIENT.
As far as I could I have fixed @dgarus comment. sc_1 == sc_2 equality
strict implementation affects performance heavily.
So now we provide these invariants sc_1.subjec,id == sc_2.subjec.id and
sc_1.subjec,login == sc_2.subjec.login
for both clients and nodes.
Permissions and permission checks provided by the SecurityContext are
orphaned. All (except one - check on node join
- but IgniteAuthenticationProcessor do not support node authentication)
permission checks are performed through IgniteSecurity#authorize.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
