Streamline Flow execution for multiple Duo providers #3498

tsschmidt · 2018-08-29T20:58:30Z

This PR refactors the Duo MFA webflow to determine the provider in an initial action that is added to the conversation scope. Subsequent actions then only need to pull the provider from the scope instead of locating it in the application context.

mmoayyed · 2018-08-29T21:16:01Z

I think this looks excellent. One comment here is that the call that puts the provider into the webflow scope is fairly generic; while duo is the only provider at this point that does in fact make use of it. It seems like either we need to move the webflow calls into the duo module and not expose generic static methods that would have one believe the API can be used to fetch a provider regardless of the module used for mfa, or we should take a pass at other providers and ensure we stuff a provider into the webflow scope for all of them.

I do like the second option myself.

tsschmidt · 2018-08-29T21:34:18Z

Are we able to set active mfa provider here: SelectiveAuthenticationProviderWebflowEventEventResolver? Then would we not need "duoInitializeLoginAction"?

mmoayyed · 2018-08-29T21:47:25Z

Yes that should work. That's the last resolver in the chain that always executes, and in fact already puts the resolved provider ids into the flow, which should make those static methods unnecessary too! You can get the provider ids that are resolved, and fetch the provider instances from the context in the right spot.

One thing that does require testing is multiple instances of duo. I believe if you have more than one defined, then the resolved provider ids that are put into the conversation-scope there include all duo ids. If that is true, then we are golden here; refactor and remove static method calls and simply use the resolved providers.

tsschmidt · 2018-08-31T01:52:22Z

Got a handle on this late in the day working with multiple Duo instances. I will try and finish up tomorrow morning and get an update committed.

… into multi-duo

tsschmidt · 2018-09-03T20:38:10Z

...ain/java/org/apereo/cas/authentication/DefaultMultifactorAuthenticationContextValidator.java

@@ -60,11 +62,14 @@
            LOGGER.debug("No multifactor authentication providers are configured");
            return Pair.of(Boolean.FALSE, Optional.empty());
        }
-        final Optional<MultifactorAuthenticationProvider> requestedProvider = locateRequestedProvider(providerMap.values(), requestedContext);
+        final Collection<MultifactorAuthenticationProvider> flattenedProviders = MultifactorAuthenticationUtils.flattenProviders(providerMap.values());


Needed to unwrap providers if Variegated Provider is found.

tsschmidt · 2018-09-03T20:39:00Z

...ain/java/org/apereo/cas/authentication/DefaultMultifactorAuthenticationContextValidator.java

@@ -137,9 +143,9 @@
            LOGGER.debug("No authentication context could be determined based on authentication attribute [{}]", this.authenticationContextAttribute);
            return null;
        }
-        contexts.forEach(context -> providers.removeIf(provider -> !provider.getId().equals(context)));


This would fail when the context contained more than one provider.

tsschmidt · 2018-09-03T20:41:08Z

...core/src/main/java/org/apereo/cas/adaptors/duo/authn/DuoAuthenticationMetaDataPopulator.java

+    private final String authenticationContextAttribute;
+
+    private final DuoAuthenticationHandler authenticationHandler;
+


Use the id of the specific provider as credential metadata so it can be matched correctly in ContextValidator.

tsschmidt · 2018-09-03T20:46:37Z

Latest commits add supporting multiple instances of Duo. The final resolved provider is decided in multiple places. If SelectiveAuthenticationProviderWebflowEventEventResolver returns multiple events then the decision is passed on to the MultfactorFactorProviderSelector that is configured. For this reason I kept the addition of adding the resoloved provider into the webflow conversation scope.

mmoayyed

If SelectiveAuthenticationProviderWebflowEventEventResolver returns multiple events then the decision is passed on to the MultfactorFactorProviderSelector that is configured.

Could you explain this a bit more? I am not sure I follow here.

mmoayyed · 2018-09-04T12:36:14Z

...ain/java/org/apereo/cas/authentication/DefaultMultifactorAuthenticationContextValidator.java

        if (!requestedProvider.isPresent()) {
            LOGGER.debug("Requested authentication provider cannot be recognized.");
            return Pair.of(Boolean.FALSE, Optional.empty());
        }
+        LOGGER.debug("RequestedContext is [{}] and Available Contexts are [{}]", requestedContext, contexts);


LOGGER.debug("Requested authentication context is [{}] and available contexts are [{}]", requestedContext, contexts);

mmoayyed · 2018-09-04T12:39:02Z

...pport-duo-core/src/main/java/org/apereo/cas/adaptors/duo/authn/DuoAuthenticationHandler.java

-            throw new IllegalArgumentException("No multifactor providers are found in the current request context");
-        }
-        final MultifactorAuthenticationProvider pr = providers.iterator().next();
+        final MultifactorAuthenticationProvider pr = WebUtils.getActiveMultifactorAuthenticationProvider(requestContext);


This is a good starting point, but at some point, we should get this cleaned up. A handler component should not try to reach beyond its limits and deal with webflow scopes. We can clean up this bit later.

I believe I was able to clear this up by not using VariegatedMFA Provider and registering multiple authentication handlers into the application context with their specific provider passed to each in the constructor. Submitting it this way for now to limit the scope of changes for a single PR.

mmoayyed · 2018-09-04T12:40:18Z

...pport-duo-core/src/main/java/org/apereo/cas/adaptors/duo/authn/DuoAuthenticationHandler.java

+     *
+     * @return - the provider id
+     */
+    public String getProviderId() {


Unless you mean to make the method private of sorts, it would be better if you actually returned the provider instance, and renamed the method to reflect that bit too.

Used currently in DuoAuthenticationMetadaPopulator. could be made protected and still work for the current solution.

mmoayyed · 2018-09-04T12:42:26Z

...apereo/cas/adaptors/duo/config/DuoSecurityAuthenticationEventExecutionPlanConfiguration.java

    }

    @RefreshScope
    @Bean
-    public AuthenticationHandler duoAuthenticationHandler() {
+    public DuoAuthenticationHandler duoAuthenticationHandler() {


I understand why you had to do this, but this needs to be re-worked. We should try to stick to the most generic type possible, and introduce things that can be polymorphically recognized.

Agreed, there are probably better ways to handle, and made easier if some other decisions are made like dropping VariegatedMFAProvider.

tsschmidt · 2018-09-04T15:28:12Z

if SelectiveAuthenticationProviderWebflowEventEventResolver returns multiple events then the decision is passed on to the MultfactorFactorProviderSelector that is configured.

I think the scenario is mfa-duo-1 is resolved as the global MFA Provider, but the service is MFA policy resolves to mfa-duo-2. Then both are returned from Selective resolver and the decision is then made by the configured MultifactorProviderSelector for the MFA flow. There are currently two instances: RankedMFAProviderSelector and GroovyMFAProviderSelector.

mmoayyed · 2018-09-04T19:15:10Z

Then both are returned from Selective resolver and the decision is then made by the configured MultifactorProviderSelector for the MFA flow.

Right. So how about we simply move the putActiveMultifactorAuthenticationProvider right after the selector picks a provider? That way, the behavior applies to all MFA providers, and isn't just a feature of duo.

tsschmidt · 2018-09-04T19:55:58Z

If Selective resolves to only one provider, then the method in CAS resolver where the Selector is executed is not called. So we would need to put the webutils put call in multiple places. I am working on a different approach that would solve this.

…

On Tue, Sep 4, 2018, 12:15 PM Misagh Moayyed ***@***.***> wrote: Then both are returned from Selective resolver and the decision is then made by the configured MultifactorProviderSelector for the MFA flow. Right. So how about we simply move the putActiveMultifactorAuthenticationProvider right after the selector picks a provider? That way, the behavior applies to all MFA providers, and isn't just a feature of duo. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#3498 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AS0kvVLlCWt31Pto_38t2B_0tTWFpbK1ks5uXtFOgaJpZM4WSUPR> .

mmoayyed · 2018-09-04T20:42:57Z

That makes sense. Thanks!

… into multi-duo

tsschmidt · 2018-09-05T19:56:17Z

The latest commit removes the use of the Variegated provider from Duo in favor of registering a Bean for each provider created in to the ApplicationContext. This also shifts the resolving of handling and providers to a place where other MFA providers can use the same configuration to offer multiple instances. Key additions and changes are:

MfaCredential
This is an interface that can be implemented to mark the Credential as a created from a MFA provider.
Adds getter/setter requirements for the provider id that created the credential.
MfaAuthenticationHandler
This is an interface used to mark the AuthenticationHandler that it used to authentic instances of MfaCredential. Adds getter for provider id and a default supports() method that checks if the passed Credential was created by the same provider the AuthenticationHandler uses
ByCredentialTypeAuthenticationHandlerResolver
Added implementation for resolve() that filters the list of handlers by passing the credential to the handler supports() method.
MfaInitializeAction
This action will pull the corresponding MultifactorAuthenticationProvider from the ApplicationContext for the active flow.
AbstractCasMultifactorWebflow
Added an ActionState to call MfaInitializeAction in the MFA based on on STAT_ID_INIT_LOGIN_FORM

Remove activeMultifactorProvider form WebUtils

mmoayyed · 2018-09-06T09:06:01Z

Nice. A few comments here:

The latest commit removes the use of the Variegated provider from Duo in favor of registering a Bean for each provider created in to the ApplicationContext.

Do we need to remove the Variegated provider API altogether then, if we have no use for it?

MfaCredential
MfaAuthenticationHandler

I am guessing that these two need to be implemented by all credentials and providers for MFA, and not just duo, right?

AbstractCasMultifactorWebflow
Added an ActionState to call MfaInitializeAction in the MFA based on on STAT_ID_INIT_LOGIN_FORM

How does this point in the webflow deal with this? Does it impact the REST API when MFA is attempted?

Also I have not looked too closely yet but we should be mindful of the scope here and how it might fit into a patch release. Are there any breaking changes, configuration or API-wise?

mmoayyed · 2018-09-06T09:10:55Z

...-support-actions/src/main/java/org/apereo/cas/web/config/CasSupportActionsConfiguration.java

@@ -273,4 +274,9 @@ public Action serviceWarningAction() {
        return new ServiceWarningAction(centralAuthenticationService, authenticationSystemSupport,
            ticketRegistrySupport, warnCookieGenerator.getIfAvailable(), principalElectionStrategy);
    }
+
+    @Bean
+    public Action mfaInitializeAction() {


Also not sure if this belongs here. Shouldn't the MFA-related actions, etc be housed in a module that is only required for MFA, and not by default?

Should we do the same with cas-sever-core-webflow-api, that has MFA implementations? Create cas-server-core-webflow-mfa and cas-server-core-weblfow-mfa-api and refactor all MFA classes to these two?

mmoayyed · 2018-09-06T09:11:13Z

...ver-support-actions/src/main/java/org/apereo/cas/web/flow/login/mfa/MfaInitializeAction.java

+ * @author Travis Schmidt
+ * @since 5.3.4
+ */
+public class MfaInitializeAction extends AbstractAction {


Same as above, really. This probably needs to move into some sort of common MFA module.

mmoayyed · 2018-09-06T09:11:38Z

...ver-support-actions/src/main/java/org/apereo/cas/web/flow/login/mfa/MfaInitializeAction.java

+    @Override
+    protected Event doExecute(final RequestContext context) throws Exception {
+        final String activeFlow = context.getActiveFlow().getId();
+        final MultifactorAuthenticationProvider provider =


This does need to move into MFA Utils (I think there is a similar call in there already)

I will look into this along with the entire MFA refactor, MFAUtils not currently in the build for this module.

mmoayyed · 2018-09-06T09:13:08Z

...erver-core-api-authentication/src/main/java/org/apereo/cas/authentication/MfaCredential.java

+     *
+     * @return - the provider id
+     */
+    String getProviderId();


This needs to be renamed to be more explicit because Credential in master presents a property for source and it would be confusing to have both source and providerId in the same class. Something slightly more verbose like MultifactorProviderId would do, I think.

Sounds good

mmoayyed · 2018-09-06T09:14:01Z

...rver-support-duo-core-mfa/src/main/java/org/apereo/cas/adaptors/duo/authn/DuoCredential.java

@@ -23,20 +24,33 @@
 @NoArgsConstructor
 @AllArgsConstructor
 @EqualsAndHashCode(of = {"username"})
-public class DuoCredential implements Credential {
+public class DuoCredential implements Credential, MfaCredential {


As noted early, this needs to be implemented by every form of MFA credential and not just Duo.

mmoayyed · 2018-09-06T09:14:53Z

...pport-duo-core/src/main/java/org/apereo/cas/adaptors/duo/authn/DuoAuthenticationHandler.java

-            || credential instanceof DuoDirectCredential;
+        return (DuoCredential.class.isAssignableFrom(credential.getClass())
+                || credential instanceof DuoDirectCredential)
+                && supports((MfaCredential) credential);


Is supports((MfaCredential) credential); a recursive call that would cause an overflow issue?

Not that I found in my testing. It calls the overloaded default method in MfaAuthenticationHandler interface.

mmoayyed · 2018-09-06T09:17:18Z

...core/src/main/java/org/apereo/cas/adaptors/duo/authn/DuoAuthenticationMetaDataPopulator.java

+public class DuoAuthenticationMetaDataPopulator extends BaseAuthenticationMetaDataPopulator {
+    private final String authenticationContextAttribute;
+
+    private final DuoAuthenticationHandler authenticationHandler;


Could this be refactored now to use a more generic type? Since you're only adding a name here? And is this not a duplicate of AuthenticationContextAttributeMetaDataPopulator?

Yes, it has been refactored into a duplicate of AuthenticationContextAttributeMetaDataPopulator as I worked through the changes. I will remove it.

mmoayyed · 2018-09-06T09:19:56Z

...apereo/cas/adaptors/duo/config/DuoSecurityAuthenticationEventExecutionPlanConfiguration.java

-            throw new IllegalArgumentException("At least one Duo instance must be defined");
-        }
-        return provider;
+    public DefaultDuoMultifactorAuthenticationProvider duoMultifactorAuthenticationProviders(final DuoSecurityMultifactorProperties duo) {


Switch the type to DuoMultifactorAuthenticationProvider? Make the method private?

mmoayyed · 2018-09-06T09:20:14Z

...apereo/cas/adaptors/duo/config/DuoSecurityAuthenticationEventExecutionPlanConfiguration.java

+        duoP.setBypassEvaluator(MultifactorAuthenticationUtils.newMultifactorAuthenticationProviderBypass(duo.getBypass()));
+        duoP.setOrder(duo.getRank());
+        duoP.setId(duo.getId());
+        applicationContext.getBeanFactory().registerSingleton(duoP.getId()+"-provider", duoP);


Use ApplicationContextProvider.registerBeanIntoApplicationContext?

I can't find that method.

I might have been looking at master. never mind this :)

mmoayyed · 2018-09-06T09:20:58Z

...apereo/cas/adaptors/duo/config/DuoSecurityAuthenticationEventExecutionPlanConfiguration.java

    }

    @RefreshScope
    @Bean
-    public AuthenticationHandler duoAuthenticationHandler() {
+    public Collection<DuoAuthenticationHandler> duoAuthenticationHandler() {


Collection<AuthenticationHandler>. Always stick to the more generic type, where possible.

tsschmidt · 2018-09-06T15:31:05Z

Do we need to remove the Variegated provider API altogether then, if we have no use for it?

Duo was the only authn method currently using it. If this approach is preferred then I believe it can be removed if we see no future use for it.

I am guessing that these two need to be implemented by all credentials and providers for MFA, and not just duo, right?

It would need to be implemented by all MFA providers that are able to be configure multiple instances of itself. So far that is only Duo. If it makes no sense to provide multiple instances of provider or the provider just can't support it then there is no need to implement these interfaces

How does this point in the webflow deal with this? Does it impact the REST API when MFA is attempted?

So non browser Duo use should work as it is currently coded. The REST API, I will need to look into further.

For breaking changes, if we only do this for Duo in the current releases then there are no breaking changes, that I foresee. Duo configuration already supports defining multiple instances and that is the only breaking change I see if we added to other MFA providers(i.e. authy[0] is not supported).

Scope wise, only applying this PR to Duo still fits within expected behavior of current releases, and only adds the functionality that rank order among Duo instances is now respected and guaranteed. Applying this pattern to other MFA providers should be handled in future releases, but this would give someone a template to use if they needed to implement this behavior locally.

mmoayyed · 2018-09-06T15:57:18Z

All sounds good.

I would leave the variegated API for now, since technically it cannot be removed in a patch release. But for the master branch, we should try to do without it.
Given your description of MfaCredential (and like the related handler), I suggest these components need to then be renamed to explain why and how someone should implement them so the name explicitly indicates objective and intent. Right now, the name suggests that if a credential is related to mfa functionality, then it may be an implementor of this MfaCredential which isn't right by design per your description. This is not an issue really, as picking the right name to convey this sort of thing is difficult and if there are no good candidates, we should remember to revisit the naming scheme for this classes.

tsschmidt · 2018-09-06T18:41:59Z

Latest commit address comments in the last review.

I was not able to find any affect on the REST API, as far as I can tell.

mmoayyed · 2018-09-06T21:39:22Z

Looks like all things pass here. Thanks very much! Excellent work. Did you want to handle the master changeset as well? Leave that up to me? Both? :)

tsschmidt · 2018-09-06T21:40:04Z

I am starting the master refactor now, waiting for IDE gradle refresh, Pushing one more commit to fix Codacy issues

Includes multi-duo refactor from PR #3498

Fixed Typo

tsschmidt · 2018-09-11T15:54:09Z

Ability to rank MFA providers of the same type.

tsschmidt · 2018-09-11T15:54:11Z

Ability to rank MFA providers of the same type.

… into multi-duo

mmoayyed · 2018-09-13T10:52:11Z

* What went wrong:
Execution failed for task ':support:cas-server-support-shibboleth:findbugsMain'.
> FindBugs rule violations were found. See the report at: file:///home/travis/build/apereo/cas/support/cas-server-support-shibboleth/build/findbugsReports/main.html
* Try:
Run with --info or --debug option to get more log output. Run with --scan to get full insights.
* Exception is:
org.gradle.api.tasks.TaskExecutionException: Execution failed for task ':support:cas-server-support-shibboleth:findbugsMain'.
	at org.gradle.api.internal.tasks.execution.ExecuteActionsTaskExecuter.executeActions(ExecuteActionsTaskExecuter.java:110)
	at org.gradle.api.internal.tasks.execution.ExecuteActionsTaskExecuter.execute(ExecuteActionsTaskExecuter.java:77)
	at org.gradle.api.internal.tasks.execution.OutputDirectoryCreatingTaskExecuter.execute(OutputDirectoryCreatingTaskExecuter.java:51)
	at org.gradle.api.internal.tasks.execution.SkipCachedTaskExecuter.execute(SkipCachedTaskExecuter.java:105)
	at org.gradle.api.internal.tasks.execution.SkipUpToDateTaskExecuter.execute(SkipUpToDateTaskExecuter.java:59)

Probably related to #3507

* Refactor to separate MFA webflows from initial flows Includes multi-duo refactor from PR #3498 * Typo fix Refactor to prevent NoSuchElementException * Ported MFA refactor changes from PR #3518 for 5.3.x * Refactor dynamic providers to config and made sure refresh scope works * revert webapp.gradle * Refactored the MultifactorAuthenticationProvider creation * Changed mfa webflow configurer to key off of start state * Added AbstractMultifactorAuthentication Refactored AuthententicationContextAttributeMetaDataPopulator * Added Mark to identify credentials added by provider Set providerId into flowscope to look up provider * Fix Tests * Fix Tests * api-mfa refactor namoing refactor * fix case of single provider * fix webflow mfa test * fix grouper mfa test * fix openid test * Refactored MultifactorAuthenticationCredential * Refactored MultifactorAuthenticationCredential DirectCredential * updated mfa documentation * Removed Variegated provider Ported forward changes from 5.3.4 * Fix DuoAuthenticationHandler supports * Fix codacy issue * Fix webflow mfa test * Fix for validation tests * try to fix ci build * Fix surrogate mfa tests * Fix checkstyle * fix spot bugs * Add mfa to tests * Fix for multivalue auth atributes

tsschmidt added 2 commits August 29, 2018 13:51

Refactor Duo flow actions to use designated provider

a24a822

Merge branch '5.3.x' of https://github.com/apereo/cas into multi-duo

188a9ff

mmoayyed added this to the 5.3.4 milestone Aug 30, 2018

mmoayyed added Spring Webflow MFA MFA Duo Security labels Aug 30, 2018

tsschmidt added 3 commits September 1, 2018 19:20

Merge branches '5.3.x' and 'multi-duo' of https://github.com/apereo/cas…

7d2ffa3

… into multi-duo

Merge branch '5.3.x' of https://github.com/apereo/cas into multi-duo

a5d0b48

Changes made to handle multiple Duo instances

370e7c6

tsschmidt commented Sep 3, 2018

View reviewed changes

mmoayyed requested changes Sep 4, 2018

View reviewed changes

tsschmidt added 5 commits September 4, 2018 14:14

Wip

ed25c38

Refactor to remove Variageted Provider

588225d

Merge branches '5.3.x' and 'multi-duo' of https://github.com/apereo/cas…

0c2b683

… into multi-duo

Merge branch 'duo-no-vprovider' into multi-duo

5dd0933

Refactor to remove Variageted Provider

89d4d18

Cleaned webapp.gradle

349ce64

Remove activeMultifactorProvider form WebUtils

mmoayyed reviewed Sep 6, 2018

View reviewed changes

Refactor after latest code review

3a4b6e5

mmoayyed approved these changes Sep 6, 2018

View reviewed changes

Fixed Codacy issues

e7143cb

tsschmidt added a commit that referenced this pull request Sep 9, 2018

Refactor to separate MFA webflows from initial flows

85b3763

Includes multi-duo refactor from PR #3498

tsschmidt mentioned this pull request Sep 9, 2018

Refactor to separate MFA webflows from initial auth flows #3508

Merged

tsschmidt added 2 commits September 10, 2018 11:14

Refactor to prevent NoSuchElementException

e5ca69e

Fixed Typo

Got rid of unused import

39c70b0

Merge branches '5.3.x' and 'multi-duo' of https://github.com/apereo/cas…

62b0e36

… into multi-duo

Marked fields transient in shibboth entity id extractor

364f65a

mmoayyed merged commit 0becacd into 5.3.x Sep 13, 2018

mmoayyed deleted the multi-duo branch September 13, 2018 16:05

		private final String authenticationContextAttribute;

		private final DuoAuthenticationHandler authenticationHandler;

Streamline Flow execution for multiple Duo providers #3498

Streamline Flow execution for multiple Duo providers #3498

Conversation

tsschmidt commented Aug 29, 2018

mmoayyed commented Aug 29, 2018 • edited Loading

tsschmidt commented Aug 29, 2018

mmoayyed commented Aug 29, 2018 • edited Loading

tsschmidt commented Aug 31, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tsschmidt commented Sep 3, 2018

mmoayyed left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tsschmidt commented Sep 4, 2018

mmoayyed commented Sep 4, 2018

tsschmidt commented Sep 4, 2018 via email

mmoayyed commented Sep 4, 2018

tsschmidt commented Sep 5, 2018

mmoayyed commented Sep 6, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tsschmidt commented Sep 6, 2018

mmoayyed commented Sep 6, 2018

tsschmidt commented Sep 6, 2018

mmoayyed commented Sep 6, 2018

tsschmidt commented Sep 6, 2018 • edited Loading

tsschmidt commented Sep 11, 2018

tsschmidt commented Sep 11, 2018

mmoayyed commented Sep 13, 2018

mmoayyed commented Aug 29, 2018 •

edited

Loading

mmoayyed commented Aug 29, 2018 •

edited

Loading

tsschmidt commented Sep 6, 2018 •

edited

Loading