-
Notifications
You must be signed in to change notification settings - Fork 482
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
AuthRegistry Implementation #139
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,83 @@ | ||
/* | ||
* Copyright 2018 The Data-Portability Project Authors. | ||
* | ||
* Licensed 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 | ||
* | ||
* https://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.dataportabilityproject.gateway; | ||
|
||
import com.google.common.base.Preconditions; | ||
import com.google.common.collect.ImmutableMap; | ||
import com.google.common.collect.ImmutableSet; | ||
import java.util.List; | ||
import java.util.Map; | ||
import org.dataportabilityproject.spi.gateway.auth.AuthDataGenerator; | ||
import org.dataportabilityproject.spi.gateway.auth.AuthServiceProvider; | ||
import org.dataportabilityproject.spi.gateway.auth.AuthServiceProviderRegistry; | ||
import org.dataportabilityproject.types.transfer.PortableType; | ||
|
||
public class PortabilityAuthServiceProviderRegistry implements AuthServiceProviderRegistry { | ||
|
||
private final ImmutableMap<String, AuthServiceProvider> authServiceProviderMap; | ||
private final ImmutableSet<String> supportedImportTypes; | ||
private final ImmutableSet<String> supportedExportTypes; | ||
|
||
// The parameters to the constructor are provided via dependency injection | ||
public PortabilityAuthServiceProviderRegistry(List<String> enabledServices, | ||
Map<String, AuthServiceProvider> serviceProviderMap){ | ||
ImmutableMap.Builder<String, AuthServiceProvider> serviceProviderBuilder = ImmutableMap | ||
.builder(); | ||
ImmutableSet.Builder<String> supportedImportTypesBuilder = ImmutableSet.builder(); | ||
ImmutableSet.Builder<String> supportedExportTypesBuilder = ImmutableSet.builder(); | ||
|
||
for(String service : enabledServices){ | ||
AuthServiceProvider authServiceProvider = serviceProviderMap.get(service); | ||
Preconditions.checkArgument(authServiceProvider != null, "AuthServiceProvider not found for [%s]", service); | ||
|
||
List<String> importTypes = authServiceProvider.getImportTypes(); | ||
List<String> exportTypes = authServiceProvider.getExportTypes(); | ||
|
||
for (String type : importTypes) { | ||
Preconditions.checkArgument(exportTypes.contains(type), | ||
"TransferDataType [%s] is available for import but not export in [%s] AuthServiceProvider", | ||
type, service); | ||
supportedImportTypesBuilder.add(type); | ||
} | ||
|
||
supportedExportTypesBuilder.addAll(exportTypes); | ||
serviceProviderBuilder.put(service, authServiceProvider); | ||
} | ||
|
||
authServiceProviderMap = serviceProviderBuilder.build(); | ||
supportedImportTypes = supportedImportTypesBuilder.build(); | ||
supportedExportTypes = supportedExportTypesBuilder.build(); | ||
} | ||
|
||
@Override | ||
public AuthDataGenerator getAuthDataGenerator(String serviceId, String transferDataType, AuthMode mode) { | ||
AuthServiceProvider provider = authServiceProviderMap.get(serviceId); | ||
Preconditions.checkArgument(provider!=null, "AuthServiceProvider not found for serviceId [%s]", serviceId); | ||
switch(mode) { | ||
case EXPORT: | ||
Preconditions.checkArgument(supportedExportTypes.contains(transferDataType), "AuthMode [%s] not valid for TransferDataType [%s]", mode, transferDataType); | ||
break; | ||
case IMPORT: | ||
Preconditions.checkArgument(supportedImportTypes.contains(transferDataType), "AuthMode [%s] not valid for TransferDataType [%s]", mode, transferDataType); | ||
break; | ||
default: | ||
throw new IllegalArgumentException("AuthMode [" + mode + "] not supported"); | ||
} | ||
|
||
return provider.getAuthDataGenerator(transferDataType, mode); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,63 @@ | ||
/* | ||
* Copyright 2018 The Data-Portability Project Authors. | ||
* | ||
* Licensed 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 | ||
* | ||
* https://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.dataportabilityproject.spi.gateway.auth; | ||
|
||
import static org.mockito.Mockito.mock; | ||
import static org.mockito.Mockito.when; | ||
|
||
import com.google.common.collect.ImmutableList; | ||
import com.google.common.collect.ImmutableMap; | ||
import java.util.List; | ||
import org.dataportabilityproject.gateway.PortabilityAuthServiceProviderRegistry; | ||
import org.junit.Rule; | ||
import org.junit.Test; | ||
import org.junit.rules.ExpectedException; | ||
|
||
public class PortabilityAuthServiceProviderRegistryTest { | ||
|
||
@Rule | ||
public ExpectedException thrown = ExpectedException.none(); | ||
|
||
@Test | ||
public void requireImportAndExportTest(){ | ||
List<String> supportedImportTypes = ImmutableList.of("photos", "contacts"); | ||
List<String> supportedExportTypes = ImmutableList.of("contacts"); | ||
|
||
AuthServiceProvider mockAuthProvider = mock(AuthServiceProvider.class); | ||
when(mockAuthProvider.getExportTypes()).thenReturn(supportedExportTypes); | ||
when(mockAuthProvider.getImportTypes()).thenReturn(supportedImportTypes); | ||
when(mockAuthProvider.getServiceId()).thenReturn("mockAuthProvider"); | ||
thrown.expect(IllegalArgumentException.class); | ||
thrown.expectMessage("available for import but not export"); | ||
|
||
AuthServiceProviderRegistry registry = new PortabilityAuthServiceProviderRegistry(ImmutableList.of("mockServiceProvider"), | ||
ImmutableMap.of("mockServiceProvider", mockAuthProvider)); | ||
} | ||
|
||
@Test | ||
public void serviceProviderNotFoundTest(){ | ||
AuthServiceProvider mockAuthProvider = mock(AuthServiceProvider.class); | ||
when(mockAuthProvider.getServiceId()).thenReturn("mockAuthProvider"); | ||
|
||
thrown.expect(IllegalArgumentException.class); | ||
thrown.expectMessage("AuthServiceProvider not found"); | ||
|
||
AuthServiceProviderRegistry registry = new PortabilityAuthServiceProviderRegistry(ImmutableList.of("ServiceDoesNotExist"), | ||
ImmutableMap.of("mockServiceProvider", mockAuthProvider)); | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,11 +4,25 @@ | |
* Manages {@link AuthServiceProvider}s registered in the system. | ||
*/ | ||
public interface AuthServiceProviderRegistry { | ||
/** | ||
* Lookup the @code{AuthDataGenerator} corresponding to the serviceId, transferType and AuthMode | ||
* specified | ||
* | ||
* @param serviceId the AuthServiceProvider to use | ||
* @param transferDataType The TransferdataType to authorize for | ||
* @param mode The authorization mode | ||
* @return An AuthDataGenerator from the specified AuthServiceProvider for the type and mode | ||
* requested. | ||
*/ | ||
AuthDataGenerator getAuthDataGenerator(String serviceId, String transferDataType, AuthMode mode); | ||
|
||
/** | ||
* Returns the provider that supports the service id. | ||
* | ||
* @param serviceId the service id | ||
*/ | ||
AuthServiceProvider getServiceProvider(String serviceId); | ||
/** | ||
* The AuthorizationMode to use for lookups. | ||
* IMPORT specifies an authorization that allows you to import a type (implying read-write permissions) | ||
* EXPORT specifies an authorization that allows you to export a type (implying read-only permissions) | ||
*/ | ||
enum AuthMode { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't mind going with AuthMode but the bigger question seems that it goes unused except in log statements. Can we remove it alltogether? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was thinking it will get passed into the getAuthDataGenerator call to the AuthServiceProvider, which will then return a generator for the correct permissions mode. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Per offline discussion, SGTM There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done - Added test. Also for posterity, this is meant to mimic the existing ServiceMode enum here: https://github.com/google/data-portability/blob/master/portability-core/src/main/java/org/dataportabilityproject/shared/ServiceMode.java Used to determine scopes for auth generation (see GoogleServiceProvider for example: https://github.com/google/data-portability/blob/master/portability-core/src/main/java/org/dataportabilityproject/serviceProviders/google/GoogleServiceProvider.java#L64) |
||
IMPORT, | ||
EXPORT | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to put values here and below?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left it blank since nothing was implemented yet for it but I didnt want to return null at the same time. I guess I could throw an Unsupported exception too. What makes the most sense?