lidavidm commented on a change in pull request #7994: URL: https://github.com/apache/arrow/pull/7994#discussion_r513761908
########## File path: java/flight/flight-core/src/main/java/org/apache/arrow/flight/RequestContext.java ########## @@ -0,0 +1,51 @@ +/* + * 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.arrow.flight; + +import java.util.Set; + +/** + * Tracks variables about the current request. + */ +public interface RequestContext { + /** + * Register a variable and a value. + * @param key the variable name. + * @param value the value. + */ + void put(String key, String value); + + /** + * Retrieve a registered variable. + * @param key the variable name. + * @return the value, or empty string if not found. Review comment: I think this should be null if not found? ########## File path: java/flight/flight-core/src/main/java/org/apache/arrow/flight/auth2/BasicCallHeaderAuthenticator.java ########## @@ -0,0 +1,102 @@ +/* + * 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.arrow.flight.auth2; + +import java.io.UnsupportedEncodingException; +import java.nio.charset.StandardCharsets; +import java.util.Base64; +import java.util.Optional; + +import org.apache.arrow.flight.CallHeaders; +import org.apache.arrow.flight.CallStatus; +import org.apache.arrow.flight.FlightRuntimeException; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * A ServerAuthHandler for username/password authentication. + */ +public class BasicCallHeaderAuthenticator implements CallHeaderAuthenticator { + + private static final Logger logger = LoggerFactory.getLogger(BasicCallHeaderAuthenticator.class); + private final BasicAuthValidator authValidator; + + public BasicCallHeaderAuthenticator(BasicAuthValidator authValidator) { + super(); + this.authValidator = authValidator; + } + + @Override + public AuthResult authenticate(CallHeaders headers) { + final String authEncoded = AuthUtilities.getValueFromAuthHeader(headers, Auth2Constants.BASIC_PREFIX); + if (authEncoded == null) { + throw CallStatus.UNAUTHENTICATED.toRuntimeException(); + } + + try { + // The value has the format Base64(<username>:<password>) + final String authDecoded = new String(Base64.getDecoder().decode(authEncoded), StandardCharsets.UTF_8); + final int colonPos = authDecoded.indexOf(':'); + if (colonPos == -1) { + throw CallStatus.UNAUTHORIZED.toRuntimeException(); Review comment: This is dealing with AuthN, so any use of Unauthorized here should be Unauthenticated ########## File path: java/flight/flight-core/src/main/java/org/apache/arrow/flight/grpc/RequestContextAdapter.java ########## @@ -0,0 +1,57 @@ +/* + * 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.arrow.flight.grpc; + +import java.util.HashMap; +import java.util.Set; + +import org.apache.arrow.flight.RequestContext; + +import io.grpc.Context; + + +/** + * Adapter for holding key value pairs. + */ +public class RequestContextAdapter implements RequestContext { + private final HashMap<String, String> map = new HashMap<>(); Review comment: nit: put the instance variables below the static variables ########## File path: java/flight/flight-core/src/main/java/org/apache/arrow/flight/auth2/BearerTokenAuthHandler.java ########## @@ -0,0 +1,60 @@ +/* + * 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.arrow.flight.auth2; + +import java.util.Optional; + +import org.apache.arrow.flight.CallHeaders; +import org.apache.arrow.flight.CallStatus; + +/** + * Partial implementation of ServerAuthHandler for bearer-token based authentication. + */ +abstract class BearerTokenAuthHandler implements CallHeaderAuthenticator { + @Override + public AuthResult authenticate(CallHeaders headers) { + final String bearerToken = AuthUtilities.getValueFromAuthHeader(headers, Auth2Constants.BEARER_PREFIX); + if (bearerToken == null) { + throw CallStatus.UNAUTHENTICATED.toRuntimeException(); + } + + if (!validateBearer(bearerToken)) { + throw CallStatus.UNAUTHORIZED.toRuntimeException(); Review comment: Ditto here, I think this should be Unauthenticated (we can't validate who you are) ########## File path: java/flight/flight-core/src/main/java/org/apache/arrow/flight/auth2/CallHeaderAuthenticator.java ########## @@ -0,0 +1,98 @@ +/* + * 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.arrow.flight.auth2; + +import java.util.Optional; + +import org.apache.arrow.flight.CallHeaders; +import org.apache.arrow.flight.FlightRuntimeException; + +/** + * Interface for Server side authentication handlers. Review comment: While the details are in the proposal, I think it'd help to document the expected auth flow for any implementers here ########## File path: java/flight/flight-core/src/main/java/org/apache/arrow/flight/auth2/CallHeaderAuthenticator.java ########## @@ -0,0 +1,98 @@ +/* + * 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.arrow.flight.auth2; + +import java.util.Optional; + +import org.apache.arrow.flight.CallHeaders; +import org.apache.arrow.flight.FlightRuntimeException; + +/** + * Interface for Server side authentication handlers. + */ +public interface CallHeaderAuthenticator { + /** + * The result of the server analyzing authentication headers. + */ + interface AuthResult { + /** + * The peer identity that was determined by the handshake process based on the + * authentication credentials supplied by the client. + * + * @return The peer identity. + */ + String getPeerIdentity(); + + /** + * The bearer token that was generated by the handshake process if applicable. + * + * @return bearer token, or Optional.empty() if bearer tokens are not supported by the auth mechanism. + */ + default Optional<String> getBearerToken() { + return Optional.empty(); + } + } + + /** + * Validate the auth headers sent by the client. + * + * @param headers The headers to authenticate. + * @return a handshake result containing a peer identity and optionally a bearer token. + * @throws FlightRuntimeException with CallStatus.UNAUTHENTICATED if credentials were not supplied + * or CallStatus.UNAUTHORIZED if credentials were supplied but were not valid. Review comment: Ditto here - Unauthorized is about whether an authenticated peer has permissions to do something, not whether their credentials were valid in the first place (sorry, I'm being a bit pedantic here, but want to make sure the two are separate) ---------------------------------------------------------------- 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]
