dimas-b commented on code in PR #499: URL: https://github.com/apache/polaris/pull/499#discussion_r1866551083
########## polaris-service/src/main/java/org/apache/polaris/service/auth/AuthenticatedPolarisPrincipalImpl.java: ########## @@ -0,0 +1,49 @@ +/* + * 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.polaris.service.auth; + +import java.util.Set; +import org.apache.polaris.core.auth.AuthenticatedPolarisPrincipal; + +public final class AuthenticatedPolarisPrincipalImpl implements AuthenticatedPolarisPrincipal { + private final long id; + private final String name; + private final Set<String> roles; + + public AuthenticatedPolarisPrincipalImpl(long id, String name, Set<String> roles) { Review Comment: > If we remove the check for the grant records, then a caller could pass in whatever scope they want and be able to assume that role without confirmation they actually have access to that role. I do not think so. The caller (Polaris API client) must obtain the credentials first. If the credentials are Polaris-owned, then the authenticator validates that the principal ID is safe to use, but it does not have to load / resolve the roles because it is not know whether the roles are needed for servicing the request. Nonetheless, the authenticity of the caller is established and roles can be validated against Polaris data. If the credentials are 3rd-party, then the IdP must have assigned them (e.g. in JWT claims). Again, the authenticator can validate the claims but does not have to resolve role names to Polaris IDs. We may need another component to deal specifically with mapping external roles / principals to Polaris entities. I would not want to overload the request authentication layer with that duty. This, I think, will complicate integration with external IdPs. For example, authenticating a JWT is in no way connected to Polaris entities. Where entities are synchronously pulled as roles from IdP or via SCIM or some other way, should not affect how credentials are verified to be trusted. -- 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]
