xiangfu0 commented on code in PR #18933: URL: https://github.com/apache/pinot/pull/18933#discussion_r3571339522
########## pinot-controller/src/main/java/org/apache/pinot/controller/api/access/SessionBasicAuthAccessControlFactory.java: ########## @@ -0,0 +1,143 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.pinot.controller.api.access; + +import java.util.Collection; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Optional; +import java.util.stream.Collectors; +import javax.ws.rs.NotAuthorizedException; +import javax.ws.rs.core.HttpHeaders; +import org.apache.pinot.common.auth.BasicAuthTokenUtils; +import org.apache.pinot.core.auth.BasicAuthPrincipal; +import org.apache.pinot.core.auth.BasicAuthPrincipalUtils; +import org.apache.pinot.core.auth.TargetType; +import org.apache.pinot.spi.env.PinotConfiguration; + + +/** + * Session-based Authentication Access Control Factory for Pinot Controller. + * + * <p><strong>NOTE: This factory is an OPTIONAL convenience class.</strong> + * The preferred approach is to use your existing factory unchanged and just add + * {@code controller.ui.session.authentication.enabled=true} to your configuration. + * That approach works with ALL auth backends (BasicAuth, ZkBasicAuth, LDAP, custom). + * + * <p>This factory is identical to {@link BasicAuthAccessControlFactory} in terms of credential + * validation (username/password via HTTP Basic Auth headers), but it reports the + * {@link AccessControl#WORKFLOW_SESSION} workflow to the UI instead of {@code BASIC}. + * + * <p>Use this factory ONLY if you cannot add + * {@code controller.ui.session.authentication.enabled=true} to your configuration + * (e.g., in environments where config changes are restricted). + * + * <p><strong>Preferred configuration (works with any factory):</strong> + * <pre> + * controller.ui.session.authentication.enabled=true + * controller.admin.access.control.factory.class=\ + * org.apache.pinot.controller.api.access.BasicAuthAccessControlFactory + * </pre> + * + * <p><strong>Alternative configuration using this factory:</strong> + * <pre> + * controller.admin.access.control.factory.class=\ + * org.apache.pinot.controller.api.access.SessionBasicAuthAccessControlFactory + * controller.admin.access.control.principals=admin,user + * controller.admin.access.control.principals.admin.password=verysecret + * </pre> + */ +public class SessionBasicAuthAccessControlFactory implements AccessControlFactory { + private static final String PREFIX = "controller.admin.access.control.principals"; + private static final String HEADER_AUTHORIZATION = "Authorization"; + + private AccessControl _accessControl; + + @Override + public void init(PinotConfiguration configuration) { + _accessControl = new SessionBasicAuthAccessControl( + BasicAuthPrincipalUtils.extractBasicAuthPrincipals(configuration, PREFIX)); + } + + @Override + public AccessControl create() { + return _accessControl; + } + + /** + * Access Control that uses Basic Auth credential validation but reports SESSION workflow. + * + * <p>When the UI calls {@code GET /auth/info}, this returns {@code {"workflow":"SESSION"}} + * which causes the UI to use POST /auth/login instead of sending Authorization headers. + */ + private static class SessionBasicAuthAccessControl implements AccessControl { + private final Map<String, BasicAuthPrincipal> _token2principal; + + SessionBasicAuthAccessControl(Collection<BasicAuthPrincipal> principals) { + _token2principal = principals.stream().collect(Collectors.toMap(BasicAuthPrincipal::getToken, p -> p)); + } + + @Override + public boolean protectAnnotatedOnly() { + return false; + } + + @Override + public boolean hasAccess(String tableName, AccessType accessType, HttpHeaders httpHeaders, String endpointUrl) { + return getPrincipal(httpHeaders) + .filter(p -> p.hasTable(tableName) && p.hasPermission(Objects.toString(accessType))).isPresent(); + } + + @Override + public boolean hasAccess(AccessType accessType, HttpHeaders httpHeaders, String endpointUrl) { + if (getPrincipal(httpHeaders).isEmpty()) { + throw new NotAuthorizedException("Basic"); + } + return true; + } + + @Override + public boolean hasAccess(HttpHeaders httpHeaders, TargetType targetType) { + return getPrincipal(httpHeaders).isPresent(); + } + + /** + * Reports SESSION workflow to the UI. + * This causes the UI to use POST /auth/login instead of sending Authorization headers. + */ + @Override + public AuthWorkflowInfo getAuthWorkflowInfo() { + return new AuthWorkflowInfo(AccessControl.WORKFLOW_SESSION); Review Comment: This factory makes `/auth/info` advertise `SESSION`, but the actual session filters only activate when `controller.ui.session.authentication.enabled=true`. With the documented alternative config that only swaps the factory, the UI will post to `/auth/login` and receive a cookie, but protected requests still won’t inject the stored Basic token because `AuthenticationFilter.isSessionEnabled()` is false. The result is a login flow that succeeds and then immediately fails protected APIs. Please make session enablement derive consistently from the workflow/factory, or remove this alternative path so operators cannot configure a broken session mode. ########## pinot-controller/src/main/resources/app/App.tsx: ########## @@ -192,6 +264,30 @@ export const App = () => { return ( <TimezoneProvider> + {/* SESSION inactivity warning dialog */} + <Dialog open={showTimeoutWarning} disableBackdropClick disableEscapeKeyDown> + <DialogTitle>Session Expiring Soon</DialogTitle> + <DialogContent> + <DialogContentText> + Your session will expire in {warningCountdown} second{warningCountdown !== 1 ? 's' : ''} due to inactivity. + </DialogContentText> + </DialogContent> + <DialogActions> + <Button + onClick={async () => { + // Slide the server-side TTL without logging out + await fetch('/auth/session', { credentials: 'include' }); Review Comment: This button says it slides the server-side TTL, but `/auth/session` only reads the fixed-expiry session and does not refresh either the server entry or the cookie Max-Age. After the original login TTL expires, active users who clicked `Stay Logged In` will still get rejected by the server. Please add a real renew path that updates both server expiry and cookie expiry, or make the UI treat this as a fixed absolute session lifetime instead of resetting the inactivity timer. -- 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. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
