Add `AcquireTokenForApp(scopes)` to `ITokenAcquisition` #39

jennyf19 · 2020-02-27T23:40:04Z

PR also includes integration tests and lab infrastructure tests

first commit has the API changes, @jmprieur please review the XML, as well as the integration test set-up
second commit has the lab infrastructure added. tests will probably fail unless ran locally.

jennyf19 · 2020-02-27T23:43:25Z

src/Microsoft.Identity.Web/ITokenAcquisition.cs

@@ -88,5 +88,16 @@ public interface ITokenAcquisition
        void ReplyForbiddenWithWwwAuthenticateHeader(
            IEnumerable<string> scopes,
            MsalUiRequiredException msalSeviceException);
+
+        /// <summary>


@jmprieur no idea what you want said here. please review. :) #Resolved

jennyf19 · 2020-02-27T23:43:40Z

src/Microsoft.Identity.Web/TokenAcquisition.cs

@@ -238,6 +237,32 @@ public class TokenAcquisition : ITokenAcquisition
            return accessToken;
        }

+        /// <summary>


@jmprieur please review comment #Resolved

jennyf19 · 2020-02-27T23:46:34Z

tests/Microsoft.Identity.Web.Test.Common/TestHelpers/InMemoryTokenCache.cs

+
+namespace Microsoft.Identity.Web.Test.Common.TestHelpers
+{
+    public class InMemoryTokenCache


this is for the lab testing #Resolved

jennyf19 · 2020-02-27T23:48:07Z

...oft.Identity.Web.Test.LabInfrastructure/Microsoft.Identity.Web.Test.LabInfrastructure.csproj

@@ -0,0 +1,16 @@
+<Project Sdk="Microsoft.NET.Sdk">


all the stuff in Microsoft.Identity.Web.Test.LabInfrastructure is copied over from the project in MSAL.NET. only a few small things were changed (mainly visual), but overall, code is the same. #Resolved

jennyf19 · 2020-02-28T00:18:20Z

src/Microsoft.Identity.Web/TokenAcquisition.cs

-
-            if (microsoftIdentityOptions.IsB2C)
+           if (string.IsNullOrEmpty(applicationOptions.Instance) &&
+                !string.IsNullOrEmpty(microsoftIdentityOptions.Instance))


This needs to be investigated. I found that the applicationOptions did not inherit from the microsoftIdentityOptions in this case, so I had to add values for both in the integration test. Issue tracked here: #37 #Resolved

separate PR and by fixing the test set-up, i no longer have the issue, but we should still look into merging the class. will update the issue.

In reply to: 385444897 [](ancestors = 385444897)

jennyf19 · 2020-02-28T00:18:37Z

src/Microsoft.Identity.Web/TokenAcquisition.cs

+
+                    app = ConfidentialClientApplicationBuilder
+                        .CreateWithApplicationOptions(applicationOptions)
+                        //.WithRedirectUri(currentUri)


//.WithRedirectUri(currentUri) [](start = 24, length = 30)

This is another issue. As we calculate the redirectUri here and then use it, if it's not registered, we fail with a null ref. Issue tracked here: #38 #Resolved

working around this for now by creating the httpcontext, but needs to be looked into (hence the issue)

In reply to: 385444973 [](ancestors = 385444973)

bgavrilMS · 2020-02-28T09:29:48Z

src/Microsoft.Identity.Web/TokenAcquisition.cs

+
+            // Use MSAL to get the right token to call the API
+            var application = GetOrBuildConfidentialClientApplication();
+            string accessToken;


nit: define and assign accessToken on the same line. #Resolved

bgavrilMS · 2020-02-28T09:31:19Z

src/Microsoft.Identity.Web/TokenAcquisition.cs

@@ -303,6 +328,7 @@ private IConfidentialClientApplication GetOrBuildConfidentialClientApplication()
        /// <returns></returns>
        private IConfidentialClientApplication BuildConfidentialClientApplication()
        {
+            string instance;
            var request = CurrentHttpContext.Request;
            var microsoftIdentityOptions = _microsoftIdentityOptions;


Why are these variables needed? Why not just work with the class memebers (_XXX) #Resolved

fixed throughout the class

In reply to: 385591146 [](ancestors = 385591146)

bgavrilMS · 2020-02-28T09:34:40Z

src/Microsoft.Identity.Web/TokenAcquisition.cs

-            IConfidentialClientApplication app = null;
-
-            if (microsoftIdentityOptions.IsB2C)
+           if (string.IsNullOrEmpty(applicationOptions.Instance) &&


I don't have the context around what applicationOptions and microsoftIdentityOptions are, but it looks to me that it violates the Single Responsability Principle. This class knows too much about options - this is a code smell that you are missing an object whose responsability is to know how to combine these options. #Resolved

agree. will do this in a separate PR

In reply to: 385592741 [](ancestors = 385592741)

bgavrilMS · 2020-02-28T09:36:06Z

src/Microsoft.Identity.Web/TokenAcquisition.cs

-                    .Build();
+                if (!applicationOptions.Instance.EndsWith("/"))
+                    applicationOptions.Instance += "/";
+                instance = applicationOptions.Instance;


If both applcationOptions and microsoft Options are defined, your instance will be NULL, probably leading to a null ref. #Resolved

n/a

In reply to: 385593451 [](ancestors = 385593451)

bgavrilMS · 2020-02-28T09:37:00Z

src/Microsoft.Identity.Web/TokenAcquisition.cs

+                        .WithB2CAuthority(authority)
+                        .Build();
+                }
+                else


what about ADFS? ADFS does not have tenant ID #WontFix

i made a separate issue for this.

In reply to: 385593891 [](ancestors = 385593891)

bgavrilMS · 2020-02-28T09:41:08Z

src/Microsoft.Identity.Web/TokenAcquisition.cs

+                }
+
+                // Initialize token cache providers
+                _tokenCacheProvider?.InitializeAsync(app.AppTokenCache);


Aren't you getting warnings about these lines? As a general rule of thumb, you should always (well 99% of time) await async operations. Otherwise you have a race condition here - you return an app and can use it to fetch tokens, but the token cache provider might not have finished initialzing.

Also, because you don't await, you will ignore possible exceptions in the InitializeAsync. #Resolved

This will of course mean that this private method also becomes Async. #Resolved

i wasn't getting warnings but have fixed all of these. good catch.

In reply to: 385595862 [](ancestors = 385595862)

bgavrilMS · 2020-02-28T09:43:41Z

src/Microsoft.Identity.Web/TokenAcquisition.cs

@@ -446,6 +492,19 @@ private IConfidentialClientApplication BuildConfidentialClientApplication()
            }
        }

+        private async Task<string> GetAccessTokenFromApplicationCacheAsync(


This method name is misleading - in case of a cache miss this goes to Evo. nit: I don't think it's even needed, it's just one line of code. #Resolved

have removed. i was hoping to isolate stuff to test, but doesn't make sense at this point.

In reply to: 385597157 [](ancestors = 385597157)

bgavrilMS · 2020-02-28T09:46:07Z

tests/Microsoft.Identity.Web.Test.Common/Mocks/MockHttpContextAccessor.cs

+
+namespace Microsoft.Identity.Web.Test.Common.Mocks
+{
+    public class MockHttpContextAccessor : IHttpContextAccessor


Why not use NSubstitute to create mocks on the fly? You will have better control over them. #Resolved

bgavrilMS · 2020-02-28T09:48:01Z

tests/Microsoft.Identity.Web.Test.Common/TestHelpers/MsalTestTokenCacheProvider.cs

+{
+    public class MsalTestTokenCacheProvider : MsalAbstractTokenCacheProvider
+    {
+        public IMemoryCache MemoryCache { get; set; }


Please review visibility of all these members. I don't think a setter is needed. #Resolved

bgavrilMS · 2020-02-28T10:03:20Z

src/Microsoft.Identity.Web/TokenAcquisition.cs

+            }
+
+            // Use MSAL to get the right token to call the API
+            var application = GetOrBuildConfidentialClientApplication();


There is already a class member named application. Please rename this class member to _application to avoid confusion. #Resolved

bgavrilMS · 2020-02-28T10:26:23Z

tests/Microsoft.Identity.Web.Test.Integration/AcquireTokenForAppIntegrationTests.cs

+        private string _ccaSecret;
+        private readonly ITestOutputHelper _output;
+
+        public AcquireTokenForAppIntegrationTests(ITestOutputHelper output) //test set-up


Have a look at NUnit's [SetUp] and [OneTimeSetup] attributes. Some of the setup should be performed once, and some of it for each test:

AcquireTokenForAppIntegrationTests -> once - I guess ctor is ok, but try to use [OneTimeSetup] as this is the recommended pattern. https://github.com/nunit/docs/wiki/SetUp-and-TearDown
InitializeTokenAcquisitionObjects -> once per test ([SetUp]) #WontFix

i did look at that, but since we are already using xunit, i just stayed with their recommendations. we can change things over though.

In reply to: 385617575 [](ancestors = 385617575)

XUnit allows the same behavior. I'll take care of it. #Resolved

thanks @pmaytak

In reply to: 385919792 [](ancestors = 385919792)

Yes, appologies, I mistakenly thought you were using NUnit instead of XUnit. #Resolved

bgavrilMS · 2020-02-28T10:28:49Z

src/Microsoft.Identity.Web/TokenAcquisition.cs

-            return app;
+            try
+            {
+                if (microsoftIdentityOptions.IsB2C)


This piece of logic is not tested. #WontFix

we are missing a lot of tests. they will come in future PRs.

In reply to: 385618752 [](ancestors = 385618752)

bgavrilMS · 2020-02-28T10:29:01Z

src/Microsoft.Identity.Web/TokenAcquisition.cs

-                    .WithRedirectUri(currentUri)
-                    .WithB2CAuthority(authority)
-                    .Build();
+                if (!microsoftIdentityOptions.Instance.EndsWith("/"))


This piece of logic is not tested. #Resolved

this is removed. not applicable anymore

In reply to: 385618845 [](ancestors = 385618845)

bgavrilMS · 2020-02-28T10:30:53Z

tests/Microsoft.Identity.Web.Test.Integration/AcquireTokenForAppIntegrationTests.cs

+            async Task result() =>
+                await _tokenAcquisition.AcquireTokenForAppAsync(TestConstants.s_scopesForUser).ConfigureAwait(false);
+
+            Exception ex = await Assert.ThrowsAsync<MsalServiceException>(result);


You can assert the error code as well if you cast directly to the type of exception:

MsalServiceExceptionex = await Assert.ThrowsAsync<MsalServiceException>(result); #Resolved

yes..thanks.

In reply to: 385619799 [](ancestors = 385619799)

tests/Microsoft.Identity.Web.Test/AadIssuerValidatorTests.cs

bgavrilMS

Generally looks good, main reason for requesting changes is not awaiting on InitializeAsync .
Could would be much cleaner and easier to test if another object would be responsible for merging microsoftOptions and appOptions.

jennyf19 · 2020-02-29T00:16:32Z

addressed your comments and/or made issues for pending items for future PRs.

In reply to: 366302244 [](ancestors = 366302244)

jmprieur

LGTM.
I've asked a couple of questions and provided a suggestion

jmprieur · 2020-02-29T00:40:54Z

src/Microsoft.Identity.Web/ITokenAcquisition.cs

@@ -88,5 +88,16 @@ public interface ITokenAcquisition
        void ReplyForbiddenWithWwwAuthenticateHeader(
            IEnumerable<string> scopes,
            MsalUiRequiredException msalSeviceException);
+
+        /// <summary>
+        /// Acquires a token from the authority configured in the app, for the confidential client itself (in the name of no user)


Is "in the name of no user" clear? should we write "not on behalf of a user" ? #Resolved

jmprieur · 2020-02-29T00:41:49Z

src/Microsoft.Identity.Web/TokenAcquisition.cs

@@ -238,6 +237,34 @@ public class TokenAcquisition : ITokenAcquisition
            return accessToken;
        }

+        /// <summary>
+        /// Acquires a token from the authority configured in the app, for the confidential client itself (in the name of no user)


same question as for the interface. #Resolved

jmprieur · 2020-02-29T00:44:54Z

tests/Microsoft.Identity.Web.Test.Common/TestHelpers/MsalTestTokenCacheProvider.cs

+{
+    public class MsalTestTokenCacheProvider : MsalAbstractTokenCacheProvider
+    {
+        public IMemoryCache MemoryCache { get; }


Should there be one IMemoryCache per cache key?
Meaning should MemoryCache be a Dictionary<string, IMemoryCache>() ? #Resolved

synced off line.

In reply to: 385982535 [](ancestors = 385982535)

- Add AcquireTokenForApp(scopes) - Add integration tests

- add await on async methods - use nsubstitute for httpcontext mock

bgavrilMS · 2020-02-29T09:42:04Z

src/Microsoft.Identity.Web/ClientInfo.cs

@@ -29,7 +29,7 @@ public static ClientInfo CreateFromJson(string clientInfo)
            {
                return DeserializeFromJson<ClientInfo>(Base64UrlHelpers.DecodeToBytes(clientInfo));
            }
-            catch (Exception exc)
+            catch (Exception)
            {
                throw new ArgumentException($"Failed to parse the returned client info. ", nameof(clientInfo));


I know this isn't your code, but ArgumentException is really not the appropriate exception here. ArgumentException tells the developer that they did smth wrong :). Parsing client_info, a piece of JSON returned by EVO, has nothing to do with this.

Personally I would just let the JSON exception flow out of the deserialize (i.e. remove the try / catch).

In general, it is a good practice to have your own dedicated exceptions for this project, similar to how MSAL has. I suggest that you review the exception strategy before releasing, as changing exceptions is breaking. CC @jmprieur #Resolved

we will discuss. thanks for putting this on our radar. i made the changes in this class that you recommend, but we will discuss how to handle the other exceptions.

In reply to: 386016125 [](ancestors = 386016125)

bgavrilMS · 2020-02-29T09:43:58Z

src/Microsoft.Identity.Web/ClientInfo.cs

-            using (var stream = new MemoryStream(jsonByteArray))
-            using (var reader = new StreamReader(stream, Encoding.UTF8))
-                return (T)JsonSerializer.Create().Deserialize(reader, typeof(T));
+            using var stream = new MemoryStream(jsonByteArray);


Fancy new C# 8 feature you're using there :) #Resolved

bgavrilMS · 2020-02-29T09:45:56Z

src/Microsoft.Identity.Web/TokenAcquisition.cs

-                    .WithRedirectUri(currentUri)
-                    .WithAuthority(authority)
-                    .Build();
+                _logger.LogInformation(ex.Message);


_logger has a LogError with an exception argument. Seems more appropriate. #Resolved

fixed. thanks.

In reply to: 386016331 [](ancestors = 386016331)

bgavrilMS · 2020-02-29T09:46:27Z

src/Microsoft.Identity.Web/TokenAcquisition.cs

+                }
+
+                // Initialize token cache providers
+                await _tokenCacheProvider.InitializeAsync(app.AppTokenCache).ConfigureAwait(false);


+1 #Resolved

…t-identity-web into jennyf/ATForApp

pmaytak · 2020-03-04T07:43:50Z

src/Microsoft.Identity.Web/TokenAcquisition.cs

                    .AcquireTokenByAuthorizationCode(scopes.Except(_scopesRequestedByMsal), context.ProtocolMessage.Code)
                    .ExecuteAsync()
                    .ConfigureAwait(false);
                context.HandleCodeRedemption(null, result.IdToken);
            }
            catch (MsalException ex)
            {
-                _logger.LogInformation(ex.Message);
+                _logger.LogInformation(ex, "Exception occured while adding an account to the cache from the auth code. ");


We should discuss putting all error strings into a separate class.

pmaytak · 2020-03-04T07:49:19Z

src/Microsoft.Identity.Web/TokenAcquisition.cs

@@ -287,61 +314,67 @@ public async Task RemoveAccountAsync(RedirectContext context)
        /// </summary>
        /// <param name="claimsPrincipal"></param>
        /// <returns></returns>
-        private IConfidentialClientApplication GetOrBuildConfidentialClientApplication()
+        private async Task<IConfidentialClientApplication> GetOrBuildConfidentialClientApplicationAsync()
        {


We should discuss renaming this method (and any ones like it). Having 'or' or 'and' in the method name suggests that a method has more than one responsibility. In this case I would probably rename it to just 'GetConfidential...' since the caller only cares about getting an app instance; whether it's built or not is internal/abstracted detail.

GetOrCreate is a pretty standard method name. In any case, this is private method, so it does not matter that much. IMO the more explicit the better.

jennyf19 requested review from bgavrilMS, jmprieur and pmaytak February 27, 2020 23:40

jennyf19 commented Feb 27, 2020

View reviewed changes

jennyf19 force-pushed the jennyf/ATForApp branch from f85a5f5 to 3ecc9f9 Compare February 27, 2020 23:50

jennyf19 changed the title ~~Add AcquireTokenForApp(scopes) to ITokenAcquisition~~ [WIP] Add AcquireTokenForApp(scopes) to ITokenAcquisition Feb 28, 2020

jennyf19 added the do not merge label Feb 28, 2020

jennyf19 commented Feb 28, 2020

View reviewed changes

bgavrilMS reviewed Feb 28, 2020

View reviewed changes

tests/Microsoft.Identity.Web.Test/AadIssuerValidatorTests.cs Show resolved Hide resolved

bgavrilMS requested changes Feb 28, 2020

View reviewed changes

jmprieur approved these changes Feb 29, 2020

View reviewed changes

jennyf19 changed the title ~~[WIP] Add AcquireTokenForApp(scopes) to ITokenAcquisition~~ Add AcquireTokenForApp(scopes) to ITokenAcquisition Feb 29, 2020

jennyf19 added In progress and removed do not merge labels Feb 29, 2020

jennyf19 added 3 commits February 28, 2020 20:38

Phase 1

cfce5df

- Add AcquireTokenForApp(scopes) - Add integration tests

Add LabInfrastructure classes from MSAL.NET repo

ad5648d

PR comments from @bgavrilMS

0b4ee95

- add await on async methods - use nsubstitute for httpcontext mock

jennyf19 force-pushed the jennyf/ATForApp branch from 1f9a5dc to 0b4ee95 Compare February 29, 2020 04:38

bgavrilMS reviewed Feb 29, 2020

View reviewed changes

bgavrilMS approved these changes Feb 29, 2020

View reviewed changes

jennyf19 added 4 commits February 29, 2020 17:28

PR comments add github action to skip integration tests

82d36a6

Merge branch 'master' into jennyf/ATForApp

8814b27

fix build

18e4af4

Merge branch 'jennyf/ATForApp' of https://github.com/AzureAD/microsof…

0aca13a

…t-identity-web into jennyf/ATForApp

jennyf19 merged commit 277cdd1 into master Mar 1, 2020

jennyf19 deleted the jennyf/ATForApp branch March 1, 2020 01:37

jennyf19 mentioned this pull request Mar 1, 2020

[Investigate] how to handle exceptions in MS identity web #47

Closed

pmaytak reviewed Mar 4, 2020

View reviewed changes

Add AcquireTokenForApp(scopes) to ITokenAcquisition #39

Add AcquireTokenForApp(scopes) to ITokenAcquisition #39

Conversation

jennyf19 commented Feb 27, 2020 • edited Loading

jennyf19 Feb 27, 2020 • edited Loading

Choose a reason for hiding this comment

jennyf19 Feb 27, 2020 • edited Loading

Choose a reason for hiding this comment

jennyf19 Feb 27, 2020 • edited Loading

Choose a reason for hiding this comment

jennyf19 Feb 27, 2020 • edited Loading

Choose a reason for hiding this comment

jennyf19 Feb 28, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jennyf19 Feb 28, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bgavrilMS Feb 28, 2020 • edited by jennyf19 Loading

Choose a reason for hiding this comment

bgavrilMS Feb 28, 2020 • edited by jennyf19 Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bgavrilMS Feb 28, 2020 • edited by jennyf19 Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bgavrilMS Feb 28, 2020 • edited by jennyf19 Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bgavrilMS Feb 28, 2020 • edited by jennyf19 Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bgavrilMS Feb 28, 2020 • edited by jennyf19 Loading

Choose a reason for hiding this comment

bgavrilMS Feb 28, 2020 • edited by jennyf19 Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bgavrilMS Feb 28, 2020 • edited by jennyf19 Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bgavrilMS Feb 28, 2020 • edited by jennyf19 Loading

Choose a reason for hiding this comment

bgavrilMS Feb 28, 2020 • edited by jennyf19 Loading

Choose a reason for hiding this comment

bgavrilMS Feb 28, 2020 • edited by jennyf19 Loading

Choose a reason for hiding this comment

bgavrilMS Feb 28, 2020 • edited by jennyf19 Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pmaytak Feb 28, 2020 • edited by jennyf19 Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bgavrilMS Feb 29, 2020 • edited by jennyf19 Loading

Choose a reason for hiding this comment

bgavrilMS Feb 28, 2020 • edited by jennyf19 Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bgavrilMS Feb 28, 2020 • edited by jennyf19 Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bgavrilMS Feb 28, 2020 • edited by jennyf19 Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bgavrilMS left a comment

Choose a reason for hiding this comment

jennyf19 commented Feb 29, 2020

jmprieur left a comment

Choose a reason for hiding this comment

jmprieur Feb 29, 2020 • edited by jennyf19 Loading

Choose a reason for hiding this comment

jmprieur Feb 29, 2020 • edited by jennyf19 Loading

Choose a reason for hiding this comment

jmprieur Feb 29, 2020 • edited by jennyf19 Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bgavrilMS Feb 29, 2020 • edited by jennyf19 Loading

Choose a reason for hiding this comment

jennyf19 Mar 1, 2020 • edited Loading

Choose a reason for hiding this comment

bgavrilMS Feb 29, 2020 • edited by jennyf19 Loading

Add `AcquireTokenForApp(scopes)` to `ITokenAcquisition` #39

Add `AcquireTokenForApp(scopes)` to `ITokenAcquisition` #39

jennyf19 commented Feb 27, 2020 •

edited

Loading

jennyf19 Feb 27, 2020 •

edited

Loading

jennyf19 Feb 27, 2020 •

edited

Loading

jennyf19 Feb 27, 2020 •

edited

Loading

jennyf19 Feb 27, 2020 •

edited

Loading

jennyf19 Feb 28, 2020 •

edited

Loading

jennyf19 Feb 28, 2020 •

edited

Loading

bgavrilMS Feb 28, 2020 •

edited by jennyf19

Loading

bgavrilMS Feb 28, 2020 •

edited by jennyf19

Loading

bgavrilMS Feb 28, 2020 •

edited by jennyf19

Loading

bgavrilMS Feb 28, 2020 •

edited by jennyf19

Loading

bgavrilMS Feb 28, 2020 •

edited by jennyf19

Loading

bgavrilMS Feb 28, 2020 •

edited by jennyf19

Loading

bgavrilMS Feb 28, 2020 •

edited by jennyf19

Loading

bgavrilMS Feb 28, 2020 •

edited by jennyf19

Loading

bgavrilMS Feb 28, 2020 •

edited by jennyf19

Loading

bgavrilMS Feb 28, 2020 •

edited by jennyf19

Loading

bgavrilMS Feb 28, 2020 •

edited by jennyf19

Loading

bgavrilMS Feb 28, 2020 •

edited by jennyf19

Loading

pmaytak Feb 28, 2020 •

edited by jennyf19

Loading

bgavrilMS Feb 29, 2020 •

edited by jennyf19

Loading

bgavrilMS Feb 28, 2020 •

edited by jennyf19

Loading

bgavrilMS Feb 28, 2020 •

edited by jennyf19

Loading

bgavrilMS Feb 28, 2020 •

edited by jennyf19

Loading

jmprieur Feb 29, 2020 •

edited by jennyf19

Loading

jmprieur Feb 29, 2020 •

edited by jennyf19

Loading

jmprieur Feb 29, 2020 •

edited by jennyf19

Loading

bgavrilMS Feb 29, 2020 •

edited by jennyf19

Loading

jennyf19 Mar 1, 2020 •

edited

Loading

bgavrilMS Feb 29, 2020 •

edited by jennyf19

Loading

bgavrilMS Feb 29, 2020 •

edited by jennyf19

Loading

bgavrilMS Feb 29, 2020 •

edited by jennyf19

Loading