Oauth2login xmlconfig implementation #7821

rh-id · 2020-01-13T22:21:44Z

Fixes gh-4557 Oauth2login xmlconfig configuration

XML configuration include the following fixes:

gh-6802: When Form Login is configured, the login page should never be
skipped

gh-6009: Encounter this issue when testing XML Configuration, set DefaultAuthenticationEventPublisher in AuthenticationManager at HttpSecurityBeanDefinitionParser class

XML configuration include the following fixes: gh-6802: When Form Login is configured, the login page should never be skipped gh-6009: Encounter this issue when testing XML Configuration, set DefaultAuthenticationEventPublisher in AuthenticationManager at HttpSecurityBeanDefinitionParser class

jgrandja

Thanks for the PR @rh-id ! Overall this looks very good. There are a couple changes to be made so please see my comments.

Also, can you ensure the copyright header is 2020 for all updated/added files and also add a test class for ClientRegistrationsBeanDefinitionParser.

Thank you!

config/src/main/java/org/springframework/security/config/http/AuthenticationConfigBuilder.java

.../src/main/java/org/springframework/security/config/http/OAuth2LoginBeanDefinitionParser.java

jgrandja · 2020-01-17T17:42:21Z

...g/springframework/security/config/oauth2/client/ClientRegistrationsBeanDefinitionParser.java

+		String beanName = parserContext.getReaderContext().generateBeanName(inMemClientRegRepoBeanDef);
+		parserContext.registerBeanComponent(new BeanComponentDefinition(inMemClientRegRepoBeanDef, beanName));
+
+		BeanDefinition inMemOAuth2AuthorizedClientServiceBeanDef = BeanDefinitionBuilder


We should first check to see if the application already registered an OAuth2AuthorizedClientService or OAuth2AuthorizedClientrepository bean before we default to the in-memory service.

this is also same problem with JwtDecoderFactory ,

Actually, this logic can be removed since OAuth2LoginBeanDefinitionParser handles the registration of OAuth2AuthorizedClientRepository

...g/springframework/security/config/oauth2/client/ClientRegistrationsBeanDefinitionParser.java

jgrandja · 2020-01-17T18:39:55Z

...test/java/org/springframework/security/config/http/OAuth2LoginBeanDefinitionParserTests.java

+				.scope(scopes).build();
+	}
+
+	public static class DummyAccessTokenResponse


Can you rename all Dummy* to either Mock* or something similar.

Alternatively, you could also use Mockito to mock some of these beans, eg.

<b:bean name="decoder" class="org.mockito.Mockito" factory-method="mock"> <b:constructor-arg value="org.springframework.security.oauth2.jwt.JwtDecoder"/> </b:bean>

jgrandja · 2020-01-17T18:42:56Z

...ework/security/config/http/OAuth2LoginBeanDefinitionParserTests-SingleClientRegistration.xml

+	</client-registrations>
+
+	<!--
+		NOTE:


Can you remove <b:import resource="userservice.xml"/>

I hit the following errors if I remove that
<failure message="org.springframework.beans.factory.BeanCreationException: Error creating bean with name 'org.springframework.security.filterChains': Cannot resolve reference to bean 'org.springframework.security.web.DefaultSecurityFilterChain#0' while setting bean property 'sourceList' with key [0]; nested exception is org.springframework.beans.factory.BeanCreationException: Error creating bean with name 'org.springframework.security.web.DefaultSecurityFilterChain#0': Cannot resolve reference to bean 'org.springframework.security.oauth2.client.web.OAuth2LoginAuthenticationFilter#0' while setting constructor argument with key [6]; nested exception is org.springframework.beans.factory.BeanCreationException: Error creating bean with name 'org.springframework.security.oauth2.client.web.OAuth2LoginAuthenticationFilter#0': Cannot resolve reference to bean 'org.springframework.security.authentication.ProviderManager#0' while setting bean property 'authenticationManager'; nested exception is org.springframework.beans.factory.BeanCreationException: Error creating bean with name 'org.springframework.security.authentication.ProviderManager#0': Cannot resolve reference to bean 'org.springframework.security.config.authentication.AuthenticationManagerFactoryBean#0' while setting constructor argument; nested exception is org.springframework.beans.factory.BeanCreationException: Error creating bean with name 'org.springframework.security.config.authentication.AuthenticationManagerFactoryBean#0': FactoryBean threw exception on object creation; nested exception is org.springframework.beans.factory.NoSuchBeanDefinitionException: No bean named 'org.springframework.security.authenticationManager' available: Did you forget to add a global <authentication-manager> element to your configuration (with child <authentication-provider> elements)? Alternatively you can use the authentication-manager-ref attribute on your <http> and <global-method-security> elements." type="org.springframework.beans.factory.BeanCreationException">org.springframework.beans.factory.BeanCreationException: Error creating bean with name 'org.springframework.security.filterChains': Cannot resolve reference to bean 'org.springframework.security.web.DefaultSecurityFilterChain#0' while setting bean property 'sourceList' with key [0]; nested exception is org.springframework.beans.factory.BeanCreationException: Error creating bean with name 'org.springframework.security.web.DefaultSecurityFilterChain#0': Cannot resolve reference to bean 'org.springframework.security.oauth2.client.web.OAuth2LoginAuthenticationFilter#0' while setting constructor argument with key [6]; nested exception is org.springframework.beans.factory.BeanCreationException: Error creating bean with name 'org.springframework.security.oauth2.client.web.OAuth2LoginAuthenticationFilter#0': Cannot resolve reference to bean 'org.springframework.security.authentication.ProviderManager#0' while setting bean property 'authenticationManager'; nested exception is org.springframework.beans.factory.BeanCreationException: Error creating bean with name 'org.springframework.security.authentication.ProviderManager#0': Cannot resolve reference to bean 'org.springframework.security.config.authentication.AuthenticationManagerFactoryBean#0' while setting constructor argument; nested exception is org.springframework.beans.factory.BeanCreationException: Error creating bean with name 'org.springframework.security.config.authentication.AuthenticationManagerFactoryBean#0': FactoryBean threw exception on object creation; nested exception is org.springframework.beans.factory.NoSuchBeanDefinitionException: No bean named 'org.springframework.security.authenticationManager' available: Did you forget to add a global <authentication-manager> element to your configuration (with child <authentication-provider> elements)? Alternatively you can use the authentication-manager-ref attribute on your <http> and <global-method-security> elements.

1. update copyright 2. add test for ClientRegistrationsBeanDefinitionParser 3. update code quality, rename fields, classes, methods 4. rename xml attributes 5. update to set request cache to redirect filter 6. update OAuth2LoginBeanDefinitionParser methods to default access 7. add client-registration-repository-ref attribute 8. add authorized-client-repository-ref attribute 9. authorized-client-service-ref attribute 10. add attribute authorization-request-repository-ref 11. update / rearrange the attributes 12. add attributes to handle authentication success and fail 13. update to set client registration issuer-uri attribute 14. add jwt-decoder-factory-ref attribute

jgrandja

Thank you for the updates @rh-id. We are getting very close to merging this. This is a significant task that you have taken on and you're doing a great job 👍 Thank you for your efforts.

I've left some further review comments. I didn't get a chance to review the tests today so I will take a look at them tomorrow and send you some more feedback.

jgrandja · 2020-01-22T16:35:36Z

...a/org/springframework/security/config/http/ClientRegistrationsBeanDefinitionParserTests.java

+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.springframework.security.config.http;


Please move to package ...config.oauth2.client

jgrandja · 2020-01-22T17:37:49Z

.../src/main/java/org/springframework/security/config/http/OAuth2LoginBeanDefinitionParser.java

+
+/**
+ * @author Ruby Hartono
+ */


Add @since 5.3

jgrandja · 2020-01-22T19:46:05Z

.../src/main/java/org/springframework/security/config/http/OAuth2LoginBeanDefinitionParser.java

+			oauth2OidcAuthProvider = oauth2OidcAuthProviderBuilder.getBeanDefinition();
+		} else {
+			oauth2OidcAuthProvider = BeanDefinitionBuilder.rootBeanDefinition(
+					"org.springframework.security.config.annotation.web.configurers.oauth2.client.OAuth2LoginConfigurer.OidcAuthenticationRequestChecker")


OidcAuthenticationRequestChecker is private so this should fail? Either way, we shouldn't reference between Java Config and XML Config. Please create an internal copy of OidcAuthenticationRequestChecker within this parser and use instead.

jgrandja · 2020-01-22T20:00:44Z

.../src/main/java/org/springframework/security/config/http/OAuth2LoginBeanDefinitionParser.java

+		BeanMetadataElement oauth2UserService = getOAuth2UserService(element);
+		BeanMetadataElement oauth2AuthRequestRepository = getOAuth2AuthorizationRequestRepository(element);
+
+		BeanDefinitionBuilder builder = BeanDefinitionBuilder.rootBeanDefinition(OAuth2LoginAuthenticationFilter.class)


Rename builder to oauth2LoginAuthenticationFilterBuilder

jgrandja · 2020-01-22T20:55:17Z

.../src/main/java/org/springframework/security/config/http/OAuth2LoginBeanDefinitionParser.java

+				.addConstructorArgValue(clientRegistrationRepository).addConstructorArgValue(authorizedClientRepository)
+				.addPropertyValue("authorizationRequestRepository", oauth2AuthRequestRepository);
+
+		String loginProcessingUrl = element.getAttribute(ATT_LOGIN_PROCESSING_URL);


We should validate loginProcessingUrl using WebConfigUtils.validateHttpRedirect() - see FormLoginBeanDefinitionParser for example

jgrandja · 2020-01-22T21:21:20Z

config/src/main/java/org/springframework/security/config/http/AuthenticationConfigBuilder.java

+	private BeanDefinition oauth2LoginEntryPoint;
+	private BeanReference oauth2LoginAuthenticationProviderRef;
+	private BeanReference oauth2LoginOidcAuthenticationProviderRef;
+
 	AuthenticationConfigBuilder(Element element, boolean forceAutoConfig,


We also need to add similar logic as OAuth2LoginConfigurer.initDefaultLoginFilter() in AuthenticationConfigBuilder.createLoginPageFilterIfNeeded(). Please add a test as well to ensure the "login links" are rendered in the default login page.

jgrandja · 2020-01-22T21:27:29Z

...g/springframework/security/config/oauth2/client/ClientRegistrationsBeanDefinitionParser.java

+
+/**
+ * @author Ruby Hartono
+ */


Add @since 5.3

jgrandja · 2020-01-22T21:35:11Z

...g/springframework/security/config/oauth2/client/ClientRegistrationsBeanDefinitionParser.java

+		String beanName = parserContext.getReaderContext().generateBeanName(inMemClientRegRepoBeanDef);
+		parserContext.registerBeanComponent(new BeanComponentDefinition(inMemClientRegRepoBeanDef, beanName));
+
+		BeanDefinition inMemOAuth2AuthorizedClientServiceBeanDef = BeanDefinitionBuilder


Actually, this logic can be removed since OAuth2LoginBeanDefinitionParser handles the registration of OAuth2AuthorizedClientRepository

jgrandja · 2020-01-22T21:39:16Z

...g/springframework/security/config/oauth2/client/ClientRegistrationsBeanDefinitionParser.java

+					? providerDetailMap.get(providerId)
+					: new HashMap<>();
+
+			Set<String> scopes = new HashSet<>(Arrays.asList(scope.split(",")));


Take a look at StringUtils and use one of those utility methods, eg. StringUtils.commaDelimitedListToSet()

jgrandja · 2020-01-22T21:44:08Z

...g/springframework/security/config/oauth2/client/ClientRegistrationsBeanDefinitionParser.java

+			String issuerUri = providerDetail.get(ATT_ISSUER_URI);
+			ClientRegistration.Builder builder = StringUtils.isEmpty(issuerUri)
+					? ClientRegistration.withRegistrationId(regId)
+					: ClientRegistrations.fromIssuerLocation(issuerUri).registrationId(regId);


ClientRegistrations.fromIssuerLocation(issuerUri) will set some defaults but the code below will override the defaults. For example, if the config did not set authorization-grant-type then authGrantType will be null which will override the default set by ClientRegistrations.fromIssuerLocation(issuerUri). Can you add a test that checks for this to ensure any values set in ClientRegistrations.fromIssuerLocation(issuerUri) are not overridden with null if the attribute was not configured.

Here is the Spring Boot specific logic to follow in OAuth2ClientPropertiesRegistrationAdapter

jgrandja

I finished reviewing the tests @rh-id. Please see my comments.

jgrandja · 2020-01-23T11:10:16Z

...test/java/org/springframework/security/config/http/OAuth2LoginBeanDefinitionParserTests.java

+	private JwtDecoderFactory<ClientRegistration> jwtDecoderFactory;
+
+	@Autowired(required = false)
+	private AuthorizationRequestRepository<OAuth2AuthorizationRequest> authorizationRequestRepository = new HttpSessionOAuth2AuthorizationRequestRepository();


Remove assignment = new HttpSessionOAuth2AuthorizationRequestRepository() since it's being @Autowired

the HttpSessionOAuth2AuthorizationRequestRepository was not registered as bean. the method OAuth2LoginBeanDefinitionParser.getOAuth2AuthorizationRequestRepository only get the bean definition but not registering it.

does this bean need to be registered? because it is not needed anywhere

does this bean need to be registered?

No, it defaults in OAuth2AuthorizationRequestRedirectFilter. However, since you're now mocking it, you need to keep it here. Just remove the default assignment (= new HttpSessionOAuth2AuthorizationRequestRepository()) since the mock will be injected.

jgrandja · 2020-01-23T13:56:03Z

...test/java/org/springframework/security/config/http/OAuth2LoginBeanDefinitionParserTests.java

+				.scope(scopes).build();
+	}
+
+	public static class MockAccessTokenResponse


We typically mock collaborators using Mockito. For example:

<b:bean name="accessTokenResponseClient" class="org.mockito.Mockito" factory-method="mock"> <b:constructor-arg value="org.springframework.security.oauth2.client.endpoint.OAuth2AccessTokenResponseClient"/> </b:bean>

And then in the test method we setup the mocks using when() and thenReturn(). We also either verify() or verifyZeroInteractions() on the collaborator to check whether it was called or not.

Can you please replace all the Mock* classes with actual Mockito mocks as per above.

FYI, you have the following test utilities for setting up the mocks:

TestClientRegistrations TestOAuth2AuthorizationRequests TestOAuth2AccessTokenResponses TestOAuth2Users TestOidcUsers TestOidcIdTokens TestJwts

jgrandja · 2020-01-23T14:12:01Z

...test/java/org/springframework/security/config/http/OAuth2LoginBeanDefinitionParserTests.java

+	}
+
+	@Test
+	public void requestWhenSingleClientRegistrationWithNonExistanceAuthenticationThenRedirectToDefaultLoginError()


This test should verify that OAuth2AuthenticationException is thrown with OAuth2Error(AUTHORIZATION_REQUEST_NOT_FOUND_ERROR_CODE). You can do this by setting up a AuthenticationFailureHandler mock and than capture the AuthenticationException argument using an ArgumentCaptor.

Let's also rename the method to requestWhenAuthorizationRequestNotFoundThenThrowAuthenticationException

jgrandja · 2020-01-23T14:20:42Z

...test/java/org/springframework/security/config/http/OAuth2LoginBeanDefinitionParserTests.java

+
+		OAuth2AuthorizationRequest authRequest = createOAuth2AuthorizationRequest(
+				clientRegistrationRepository.findByRegistrationId("google-login"));
+		authorizationRequestRepository.saveAuthorizationRequest(authRequest, request, response);


You can simplify the setup for OAuth2AuthorizationRequest by setting up a mock for authorizationRequestRepository and then:

when(authorizationRequestRepository.loadAuthorizationRequest(any())).thenReturn(authRequest)

authRequest can be created using TestOAuth2AuthorizationRequests.

NOTE: Please apply this same update to all other test methods that mock OAuth2AuthorizationRequest

jgrandja · 2020-01-23T14:23:41Z

...test/java/org/springframework/security/config/http/OAuth2LoginBeanDefinitionParserTests.java

+		this.mvc.perform(get("/login/oauth2/code/google").params(params).session(session))
+				.andExpect(status().is3xxRedirection()).andExpect(redirectedUrl("/"));
+
+		Authentication authentication = this.securityContextRepository


Instead of using securityContextRepository to verify the authentication, you can setup a AuthenticationSuccessHandler mock and then capture the Authentication using an ArgumentCaptor. This also has the benefit of verifying that the AuthenticationSuccessHandler is called.

NOTE: Please apply this same update to all other test methods that verify the Authentication

jgrandja · 2020-01-23T16:23:24Z

...a/org/springframework/security/config/http/ClientRegistrationsBeanDefinitionParserTests.java

+ * @author Ruby Hartono
+ */
+public class ClientRegistrationsBeanDefinitionParserTests {
+	private static final String CONFIG_LOCATION_PREFIX = "classpath:org/springframework/security/config/http/ClientRegistrationsBeanDefinitionParserTests";


Please move to package ...config.oauth2.client

jgrandja · 2020-01-23T16:29:45Z

...a/org/springframework/security/config/http/ClientRegistrationsBeanDefinitionParserTests.java

+	private OAuth2AuthorizedClientService oauth2AuthorizedClientService;
+
+	@Test
+	public void multiClientRegistrationConfiguration() throws Exception {


Let's rename to parseWhenMultipleClientsConfiguredThenAvailableInRepository

jgrandja · 2020-01-23T16:40:52Z

...a/org/springframework/security/config/http/ClientRegistrationsBeanDefinitionParserTests.java

+				.isEqualTo(AuthenticationMethod.HEADER);
+		assertThat(githubProviderDetails.getUserInfoEndpoint().getUserNameAttributeName()).isEqualTo("id");
+	}
+


Please also add a test for issuer-uri to trigger ClientRegistrations.fromIssuerLocation() . You could mock the OIDC Discovery Endpoint using MockWebServer - see ClientRegistrationsTest for an example

jgrandja · 2020-01-23T16:45:34Z

...ecurity/config/http/ClientRegistrationsBeanDefinitionParserTests-MultiClientRegistration.xml

+			https://www.springframework.org/schema/security/spring-security.xsd
+			http://www.springframework.org/schema/beans
+			https://www.springframework.org/schema/beans/spring-beans.xsd">
+	<client-registrations>


Instead of duplicating google and github <client-registration> in the test xml configs, let's use <b:import resource="registration.xml"/> instead. I would suggest creating google-registration.xml, github-registration.xml and google-github-registration.xml. You can then <b:import resource=../> in the xml configs.

I had applied the <b:import resource=../> in the test xml configs. there is a caveat in using import for this parser.

this is related with issue gh-5347 auto-redirect when only one client configured
See OAuth2LoginBeanDefinitionParser.getLoginEntryPoint
Element clientRegsElt = DomUtils.getChildElementByTagName(element.getOwnerDocument().getDocumentElement(),Elements.CLIENT_REGISTRATIONS);

It won't be able to find client-registrations element if this element was not configured in the same XML file.

and as we know there is now way to be able to get/check the contents of ClientRegistrationRepository bean in this parser.

Just want to update, i just found a way to get ApplicationContext and check for the ClientRegistrationRepository in which i applied it on the loginLinks (see my latest changes soon) but this will still be a problem, see method AuthenticationConfigBuilder.selectEntryPoint couldn't check ApplicationContext on that part

jgrandja · 2020-01-23T16:49:45Z

...g/springframework/security/config/oauth2/client/ClientRegistrationsBeanDefinitionParser.java

+			String clientSecret = clientRegElt.getAttribute(ATT_CLIENT_SECRET);
+			String clientAuthMethod = clientRegElt.getAttribute(ATT_CLIENT_AUTHENTICATION_METHOD);
+			String authGrantType = clientRegElt.getAttribute(ATT_AUTHORIZATION_GRANT_TYPE);
+			String redirUri = clientRegElt.getAttribute(ATT_REDIRECT_URI);


Typo -> redirectUri

rh-id · 2020-02-06T12:53:06Z

I'm not sure why travis hit error testing OAuth2LoginBeanDefinitionParserTests I execute the test in my local gradlew spring-security-config:cleanTest spring-security-config:test --tests OAuth2LoginBeanDefinitionParserTests it seemed no issue for both gradle 5.6.2 and gradle 6.1.1
am I missing anything?

jgrandja · 2020-02-07T22:15:10Z

@rh-id Don't worry about the test failure in Travis. Sometimes there are one-time glitches. I don't see an issue in the log related to your changes.

jgrandja · 2020-02-07T22:18:47Z

@rh-id Just a heads up that I would like to get this merged by end of next week ideally. I'm definitely not rushing you as I understand you have time limitations as well with your full-time job. But 5.3.0 is fast approaching and I'd like to get this merged earlier as we're considering a RC2 release which might happen in 2 weeks. I plan on doing further reviews next week and I may jump in and help finish the remaining tasks with you.

@SInCE

1. Move client registrations bean parser test 2. Add @SInCE 5.3 3. update unit test to migrate to mockito 4. update oauth2login parser to include other arguments 5. update token-uri attribute for client-registrations provider to be optional instead of required because it is conflict with the issuer-uri 6. update client registration bean definition parser,fix authenticationFailureHandler in oauth2login bean definition parser 7. configure loginLinks

rh-id · 2020-02-08T04:40:27Z

hi @jgrandja I actually finish the changes yesterday except for HttpSessionOAuth2AuthorizationRequestRepository assignment on the test 😅 . and would like to ask you to review after i solve the travis issue. I had removed the assignment on the last push.
you may proceed to review.

jgrandja · 2020-02-12T21:06:05Z

@rh-id I just merged this to master! Thank you so much for this very important contribution! There are many users that have been waiting for this and honestly it might not have made it into 5.3 if you didn't take this on.

Btw, is this your first contribution to Spring Security? Regardless, you did an amazing job! 👍 Just as an FYI, I added a polish commit. Please review and let me know if you have any questions. You can always comment on the specific line of code and I'll respond.

The OAuth2LoginBeanConfig in OAuth2LoginBeanDefinitionParser was very creative. Great idea!

If you're interested and have the time, let me know if you would like to take on #5184. You've pretty much did most of the work already since HttpSecurity.oauth2Client() is a much smaller subset of HttpSecurity.oauth2Login(). Either way, I'll look forward to your next contribution :)

rh-id · 2020-02-17T15:56:21Z

@jgrandja thank you for your detail explanation and reviews as well , I learn a lot of things from you 👍.

jgrandja · 2020-02-18T01:22:09Z

@rh-id I'm really glad you learned a lot. That was one of the goals and we achieved it 👍

jgrandja added 2 commits December 13, 2019 16:30

Define schema elements for oauth2Login()

c99289b

Add template for BeanDefinitionParser implementations

71b5571

spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jan 13, 2020

rh-id mentioned this pull request Jan 15, 2020

Add XML namespace support for oauth2Login() #7743

Closed

jgrandja self-assigned this Jan 17, 2020

jgrandja added in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Jan 17, 2020

jgrandja added this to the 5.3.0.RC1 milestone Jan 17, 2020

jgrandja requested changes Jan 17, 2020

View reviewed changes

rh-id requested a review from jgrandja January 21, 2020 12:32

jgrandja requested changes Jan 22, 2020

View reviewed changes

jgrandja requested changes Jan 23, 2020

View reviewed changes

eleftherias modified the milestones: 5.3.0.RC1, 5.3.0 Feb 5, 2020

jgrandja closed this Feb 12, 2020

rh-id deleted the oauth2login-xmlconfig branch February 17, 2020 15:40

Oauth2login xmlconfig implementation #7821

Oauth2login xmlconfig implementation #7821

Uh oh!

Conversation

rh-id commented Jan 13, 2020

Uh oh!

jgrandja left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jgrandja left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jgrandja left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jgrandja Feb 7, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jgrandja Feb 7, 2020 •

edited

Loading