From 2341dd0fa418d0aaf1da7833af4cf77e367b9e41 Mon Sep 17 00:00:00 2001 From: hiranya911 Date: Thu, 24 Jan 2019 16:48:43 -0800 Subject: [PATCH 1/4] Improvements to thread-safety and error handling --- .../Auth/FirebaseAuthTest.cs | 2 + .../FirebaseAdmin/Auth/FirebaseAuth.cs | 75 +++++++++++-------- .../FirebaseAdmin/Auth/FirebaseUserManager.cs | 72 ++++++++++-------- 3 files changed, 86 insertions(+), 63 deletions(-) diff --git a/FirebaseAdmin/FirebaseAdmin.Tests/Auth/FirebaseAuthTest.cs b/FirebaseAdmin/FirebaseAdmin.Tests/Auth/FirebaseAuthTest.cs index 491aa9af..0f6ee536 100644 --- a/FirebaseAdmin/FirebaseAdmin.Tests/Auth/FirebaseAuthTest.cs +++ b/FirebaseAdmin/FirebaseAdmin.Tests/Auth/FirebaseAuthTest.cs @@ -69,6 +69,8 @@ await Assert.ThrowsAsync( async () => await auth.CreateCustomTokenAsync("user")); await Assert.ThrowsAsync( async () => await auth.VerifyIdTokenAsync("user")); + await Assert.ThrowsAsync( + async () => await auth.SetCustomUserClaimsAsync("user", null)); } [Fact] diff --git a/FirebaseAdmin/FirebaseAdmin/Auth/FirebaseAuth.cs b/FirebaseAdmin/FirebaseAdmin/Auth/FirebaseAuth.cs index 65910fbb..0e58e94c 100644 --- a/FirebaseAdmin/FirebaseAdmin/Auth/FirebaseAuth.cs +++ b/FirebaseAdmin/FirebaseAdmin/Auth/FirebaseAuth.cs @@ -39,8 +39,8 @@ private FirebaseAuth(FirebaseApp app) () => FirebaseTokenFactory.Create(this.app), true); this.idTokenVerifier = new Lazy( () => FirebaseTokenVerifier.CreateIDTokenVerifier(this.app), true); - this.userManager = new Lazy(() => - FirebaseUserManager.Create(this.app)); + this.userManager = new Lazy( + () => FirebaseUserManager.Create(this.app), true); } /// @@ -211,17 +211,7 @@ public async Task CreateCustomTokenAsync( IDictionary developerClaims, CancellationToken cancellationToken) { - FirebaseTokenFactory tokenFactory; - lock (this.authLock) - { - if (this.deleted) - { - throw new InvalidOperationException("Cannot invoke after deleting the app."); - } - - tokenFactory = this.tokenFactory.Value; - } - + var tokenFactory = this.IfNotDeleted(() => this.tokenFactory.Value); return await tokenFactory.CreateCustomTokenAsync( uid, developerClaims, cancellationToken).ConfigureAwait(false); } @@ -268,15 +258,8 @@ public async Task VerifyIdTokenAsync(string idToken) public async Task VerifyIdTokenAsync( string idToken, CancellationToken cancellationToken) { - lock (this.authLock) - { - if (this.deleted) - { - throw new InvalidOperationException("Cannot invoke after deleting the app."); - } - } - - return await this.idTokenVerifier.Value.VerifyTokenAsync(idToken, cancellationToken) + var idTokenVerifier = this.IfNotDeleted(() => this.idTokenVerifier.Value); + return await idTokenVerifier.VerifyTokenAsync(idToken, cancellationToken) .ConfigureAwait(false); } @@ -295,22 +278,39 @@ public async Task VerifyIdTokenAsync( /// The claims to be stored on the user account, and made /// available to Firebase security rules. These must be serializable to JSON, and after /// serialization it should not be larger than 1000 characters. - public async Task SetCustomUserClaimsAsync(string uid, IReadOnlyDictionary claims) + public async Task SetCustomUserClaimsAsync( + string uid, IReadOnlyDictionary claims) { - lock (this.authLock) - { - if (this.deleted) - { - throw new InvalidOperationException("Cannot invoke after deleting the app."); - } - } + await this.SetCustomUserClaimsAsync(uid, claims, default(CancellationToken)); + } + /// + /// Sets the specified custom claims on an existing user account. A null claims value + /// removes any claims currently set on the user account. The claims should serialize into + /// a valid JSON string. The serialized claims must not be larger than 1000 characters. + /// + /// A task that completes when the claims have been set. + /// If is null, empty or longer + /// than 128 characters. Or, if the serialized is larger than 1000 + /// characters. + /// The user ID string for the custom claims will be set. Must not be null + /// or longer than 128 characters. + /// + /// The claims to be stored on the user account, and made + /// available to Firebase security rules. These must be serializable to JSON, and after + /// serialization it should not be larger than 1000 characters. + /// A cancellation token to monitor the asynchronous + /// operation. + public async Task SetCustomUserClaimsAsync( + string uid, IReadOnlyDictionary claims, CancellationToken cancellationToken) + { + var userManager = this.IfNotDeleted(() => this.userManager.Value); var user = new UserRecord(uid) { CustomClaims = claims, }; - await this.userManager.Value.UpdateUserAsync(user); + await userManager.UpdateUserAsync(user, cancellationToken).ConfigureAwait(false); } /// @@ -332,5 +332,18 @@ void IFirebaseService.Delete() } } } + + private TResult IfNotDeleted(Func func) + { + lock (this.authLock) + { + if (this.deleted) + { + throw new InvalidOperationException("Cannot invoke after deleting the app."); + } + + return func(); + } + } } } diff --git a/FirebaseAdmin/FirebaseAdmin/Auth/FirebaseUserManager.cs b/FirebaseAdmin/FirebaseAdmin/Auth/FirebaseUserManager.cs index 805d6347..8934020b 100644 --- a/FirebaseAdmin/FirebaseAdmin/Auth/FirebaseUserManager.cs +++ b/FirebaseAdmin/FirebaseAdmin/Auth/FirebaseUserManager.cs @@ -14,9 +14,11 @@ using System; using System.Net.Http; +using System.Threading; using System.Threading.Tasks; using Google.Apis.Auth.OAuth2; using Google.Apis.Http; +using Google.Apis.Json; using Newtonsoft.Json.Linq; namespace FirebaseAdmin.Auth @@ -64,22 +66,17 @@ public static FirebaseUserManager Create(FirebaseApp app) /// /// If the server responds that cannot update the user. /// The user which we want to update. - public async Task UpdateUserAsync(UserRecord user) + /// A cancellation token to monitor the asynchronous + /// operation. + public async Task UpdateUserAsync( + UserRecord user, CancellationToken cancellationToken = default(CancellationToken)) { - var updatePath = "/accounts:update"; - var resopnse = await this.PostAsync(updatePath, user); - - try - { - var userResponse = resopnse.ToObject(); - if (userResponse.Uid != user.Uid) - { - throw new FirebaseException($"Failed to update user: {user.Uid}"); - } - } - catch (Exception e) + const string updatePath = "accounts:update"; + var response = await this.PostAndDeserializeAsync( + updatePath, user, cancellationToken).ConfigureAwait(false); + if (response["localId"]?.Value() != user.Uid) { - throw new FirebaseException("Error while calling Firebase Auth service", e); + throw new FirebaseException($"Failed to update user: {user.Uid}"); } } @@ -88,31 +85,42 @@ public void Dispose() this.httpClient.Dispose(); } - private async Task PostAsync(string path, UserRecord user) + private async Task PostAndDeserializeAsync( + string path, object body, CancellationToken cancellationToken) { - var requestUri = $"{this.baseUrl}{path}"; - HttpResponseMessage response = null; try { - response = await this.httpClient.PostJsonAsync(requestUri, user, default); - var json = await response.Content.ReadAsStringAsync(); - - if (response.IsSuccessStatusCode) - { - return JObject.Parse(json); - } - else - { - var error = "Response status code does not indicate success: " - + $"{(int)response.StatusCode} ({response.StatusCode})" - + $"{Environment.NewLine}{json}"; - throw new FirebaseException(error); - } + var json = await this.PostAsync(path, body, cancellationToken) + .ConfigureAwait(false); + return NewtonsoftJsonSerializer.Instance.Deserialize(json); + } + catch (FirebaseException) + { + throw; } - catch (HttpRequestException e) + catch (Exception e) { throw new FirebaseException("Error while calling Firebase Auth service", e); } } + + private async Task PostAsync( + string path, object body, CancellationToken cancellationToken) + { + var requestUri = $"{this.baseUrl}/{path}"; + var response = await this.httpClient + .PostJsonAsync(requestUri, body, cancellationToken) + .ConfigureAwait(false); + var json = await response.Content.ReadAsStringAsync().ConfigureAwait(false); + if (!response.IsSuccessStatusCode) + { + var error = "Response status code does not indicate success: " + + $"{(int)response.StatusCode} ({response.StatusCode})" + + $"{Environment.NewLine}{json}"; + throw new FirebaseException(error); + } + + return json; + } } } From 437bd9e087e5799c9014c8ee41a2d2c175eb2cf0 Mon Sep 17 00:00:00 2001 From: hiranya911 Date: Thu, 24 Jan 2019 17:30:27 -0800 Subject: [PATCH 2/4] Updated response check --- FirebaseAdmin/FirebaseAdmin/Auth/FirebaseUserManager.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/FirebaseAdmin/FirebaseAdmin/Auth/FirebaseUserManager.cs b/FirebaseAdmin/FirebaseAdmin/Auth/FirebaseUserManager.cs index 8934020b..95732250 100644 --- a/FirebaseAdmin/FirebaseAdmin/Auth/FirebaseUserManager.cs +++ b/FirebaseAdmin/FirebaseAdmin/Auth/FirebaseUserManager.cs @@ -74,7 +74,7 @@ public async Task UpdateUserAsync( const string updatePath = "accounts:update"; var response = await this.PostAndDeserializeAsync( updatePath, user, cancellationToken).ConfigureAwait(false); - if (response["localId"]?.Value() != user.Uid) + if (user.Uid != (string)response["localId"]) { throw new FirebaseException($"Failed to update user: {user.Uid}"); } From f114f350418e1a9902a3ece2b22d12db8e07b00e Mon Sep 17 00:00:00 2001 From: hiranya911 Date: Thu, 24 Jan 2019 22:06:45 -0800 Subject: [PATCH 3/4] Cleaning up the code --- .../FirebaseAdmin/Auth/FirebaseAuth.cs | 11 ++------ .../FirebaseAdmin/Auth/FirebaseUserManager.cs | 28 ++++++++++++------- FirebaseAdmin/FirebaseAdmin/Extensions.cs | 17 ++++++++++- 3 files changed, 36 insertions(+), 20 deletions(-) diff --git a/FirebaseAdmin/FirebaseAdmin/Auth/FirebaseAuth.cs b/FirebaseAdmin/FirebaseAdmin/Auth/FirebaseAuth.cs index 0e58e94c..80c4ce3c 100644 --- a/FirebaseAdmin/FirebaseAdmin/Auth/FirebaseAuth.cs +++ b/FirebaseAdmin/FirebaseAdmin/Auth/FirebaseAuth.cs @@ -321,15 +321,8 @@ void IFirebaseService.Delete() lock (this.authLock) { this.deleted = true; - if (this.tokenFactory.IsValueCreated) - { - this.tokenFactory.Value.Dispose(); - } - - if (this.userManager.IsValueCreated) - { - this.userManager.Value.Dispose(); - } + this.tokenFactory.DisposeIfCreated(); + this.userManager.DisposeIfCreated(); } } diff --git a/FirebaseAdmin/FirebaseAdmin/Auth/FirebaseUserManager.cs b/FirebaseAdmin/FirebaseAdmin/Auth/FirebaseUserManager.cs index 95732250..b2644d60 100644 --- a/FirebaseAdmin/FirebaseAdmin/Auth/FirebaseUserManager.cs +++ b/FirebaseAdmin/FirebaseAdmin/Auth/FirebaseUserManager.cs @@ -88,28 +88,36 @@ public void Dispose() private async Task PostAndDeserializeAsync( string path, object body, CancellationToken cancellationToken) { + var json = await this.PostAsync(path, body, cancellationToken).ConfigureAwait(false); try { - var json = await this.PostAsync(path, body, cancellationToken) - .ConfigureAwait(false); return NewtonsoftJsonSerializer.Instance.Deserialize(json); } - catch (FirebaseException) - { - throw; - } catch (Exception e) { - throw new FirebaseException("Error while calling Firebase Auth service", e); + throw new FirebaseException("Error while parsing Auth service response", e); } } private async Task PostAsync( string path, object body, CancellationToken cancellationToken) { - var requestUri = $"{this.baseUrl}/{path}"; - var response = await this.httpClient - .PostJsonAsync(requestUri, body, cancellationToken) + try + { + var url = $"{this.baseUrl}/{path}"; + return await this.SendRequestAsync(url, body, cancellationToken) + .ConfigureAwait(false); + } + catch (HttpRequestException e) + { + throw new FirebaseException("Error while calling Firebase Auth service", e); + } + } + + private async Task SendRequestAsync( + string url, object body, CancellationToken cancellationToken) + { + var response = await this.httpClient.PostJsonAsync(url, body, cancellationToken) .ConfigureAwait(false); var json = await response.Content.ReadAsStringAsync().ConfigureAwait(false); if (!response.IsSuccessStatusCode) diff --git a/FirebaseAdmin/FirebaseAdmin/Extensions.cs b/FirebaseAdmin/FirebaseAdmin/Extensions.cs index 650db9df..3b25e1ea 100644 --- a/FirebaseAdmin/FirebaseAdmin/Extensions.cs +++ b/FirebaseAdmin/FirebaseAdmin/Extensions.cs @@ -14,6 +14,7 @@ using System; using System.Net.Http; +using System.Text; using System.Threading; using System.Threading.Tasks; using Google.Apis.Auth.OAuth2; @@ -93,7 +94,7 @@ public static async Task PostJsonAsync( this HttpClient client, string requestUri, T body, CancellationToken cancellationToken) { var payload = NewtonsoftJsonSerializer.Instance.Serialize(body); - var content = new StringContent(payload, System.Text.Encoding.UTF8, "application/json"); + var content = new StringContent(payload, Encoding.UTF8, "application/json"); return await client.PostAsync(requestUri, content, cancellationToken) .ConfigureAwait(false); } @@ -108,5 +109,19 @@ public static long UnixTimestamp(this IClock clock) var timeSinceEpoch = clock.UtcNow.Subtract(new DateTime(1970, 1, 1)); return (long)timeSinceEpoch.TotalSeconds; } + + /// + /// Disposes a lazy-initialized object if the object has already been created. + /// + /// The lazy initializer containing a disposable object. + /// Type of the object that needs to be disposed. + public static void DisposeIfCreated(this Lazy lazy) + where T : IDisposable + { + if (lazy.IsValueCreated) + { + lazy.Value.Dispose(); + } + } } } From 53ce449c1ec2691c29f27c4e5bf16a4ff900273f Mon Sep 17 00:00:00 2001 From: hiranya911 Date: Fri, 25 Jan 2019 10:46:10 -0800 Subject: [PATCH 4/4] Cleaned up the http client code in user manager --- .../FirebaseAdmin/Auth/FirebaseUserManager.cs | 62 +++++++++++-------- FirebaseAdmin/FirebaseAdmin/Extensions.cs | 18 +++++- 2 files changed, 51 insertions(+), 29 deletions(-) diff --git a/FirebaseAdmin/FirebaseAdmin/Auth/FirebaseUserManager.cs b/FirebaseAdmin/FirebaseAdmin/Auth/FirebaseUserManager.cs index b2644d60..75f53137 100644 --- a/FirebaseAdmin/FirebaseAdmin/Auth/FirebaseUserManager.cs +++ b/FirebaseAdmin/FirebaseAdmin/Auth/FirebaseUserManager.cs @@ -38,24 +38,23 @@ internal class FirebaseUserManager : IDisposable internal FirebaseUserManager(FirebaseUserManagerArgs args) { + if (string.IsNullOrEmpty(args.ProjectId)) + { + throw new ArgumentException( + "Must initialize FirebaseApp with a project ID to manage users."); + } + this.httpClient = args.ClientFactory.CreateAuthorizedHttpClient(args.Credential); this.baseUrl = string.Format(IdTooklitUrl, args.ProjectId); } public static FirebaseUserManager Create(FirebaseApp app) { - var projectId = app.GetProjectId(); - if (string.IsNullOrEmpty(projectId)) - { - throw new ArgumentException( - "Must initialize FirebaseApp with a project ID to manage users."); - } - var args = new FirebaseUserManagerArgs { ClientFactory = new HttpClientFactory(), Credential = app.Options.Credential, - ProjectId = projectId, + ProjectId = app.GetProjectId(), }; return new FirebaseUserManager(args); @@ -89,6 +88,11 @@ private async Task PostAndDeserializeAsync( string path, object body, CancellationToken cancellationToken) { var json = await this.PostAsync(path, body, cancellationToken).ConfigureAwait(false); + return this.SafeDeserialize(json); + } + + private TResult SafeDeserialize(string json) + { try { return NewtonsoftJsonSerializer.Instance.Deserialize(json); @@ -101,34 +105,38 @@ private async Task PostAndDeserializeAsync( private async Task PostAsync( string path, object body, CancellationToken cancellationToken) + { + var request = new HttpRequestMessage() + { + Method = HttpMethod.Post, + RequestUri = new Uri($"{this.baseUrl}/{path}"), + Content = NewtonsoftJsonSerializer.Instance.CreateJsonHttpContent(body), + }; + return await this.SendAsync(request, cancellationToken).ConfigureAwait(false); + } + + private async Task SendAsync( + HttpRequestMessage request, CancellationToken cancellationToken) { try { - var url = $"{this.baseUrl}/{path}"; - return await this.SendRequestAsync(url, body, cancellationToken) + var response = await this.httpClient.SendAsync(request, cancellationToken) .ConfigureAwait(false); + var json = await response.Content.ReadAsStringAsync().ConfigureAwait(false); + if (!response.IsSuccessStatusCode) + { + var error = "Response status code does not indicate success: " + + $"{(int)response.StatusCode} ({response.StatusCode})" + + $"{Environment.NewLine}{json}"; + throw new FirebaseException(error); + } + + return json; } catch (HttpRequestException e) { throw new FirebaseException("Error while calling Firebase Auth service", e); } } - - private async Task SendRequestAsync( - string url, object body, CancellationToken cancellationToken) - { - var response = await this.httpClient.PostJsonAsync(url, body, cancellationToken) - .ConfigureAwait(false); - var json = await response.Content.ReadAsStringAsync().ConfigureAwait(false); - if (!response.IsSuccessStatusCode) - { - var error = "Response status code does not indicate success: " - + $"{(int)response.StatusCode} ({response.StatusCode})" - + $"{Environment.NewLine}{json}"; - throw new FirebaseException(error); - } - - return json; - } } } diff --git a/FirebaseAdmin/FirebaseAdmin/Extensions.cs b/FirebaseAdmin/FirebaseAdmin/Extensions.cs index 3b25e1ea..fc1c7a56 100644 --- a/FirebaseAdmin/FirebaseAdmin/Extensions.cs +++ b/FirebaseAdmin/FirebaseAdmin/Extensions.cs @@ -93,12 +93,26 @@ public static ConfigurableHttpClient CreateAuthorizedHttpClient( public static async Task PostJsonAsync( this HttpClient client, string requestUri, T body, CancellationToken cancellationToken) { - var payload = NewtonsoftJsonSerializer.Instance.Serialize(body); - var content = new StringContent(payload, Encoding.UTF8, "application/json"); + var content = NewtonsoftJsonSerializer.Instance.CreateJsonHttpContent(body); return await client.PostAsync(requestUri, content, cancellationToken) .ConfigureAwait(false); } + /// + /// Serializes the into JSON, and wraps the result in an instance + /// of , which can be included in an outgoing HTTP request. + /// + /// An instance of containing the JSON representation + /// of . + /// The JSON serializer to serialize the given object. + /// The object that will be serialized into JSON. + public static HttpContent CreateJsonHttpContent( + this NewtonsoftJsonSerializer serializer, object body) + { + var payload = serializer.Serialize(body); + return new StringContent(payload, Encoding.UTF8, "application/json"); + } + /// /// Returns a Unix-styled timestamp (seconds from epoch) from the . ///