Skip to content

Commit 2341dd0

Browse files
committed
Improvements to thread-safety and error handling
1 parent e6bb4d2 commit 2341dd0

File tree

3 files changed

+86
-63
lines changed

3 files changed

+86
-63
lines changed

FirebaseAdmin/FirebaseAdmin.Tests/Auth/FirebaseAuthTest.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,8 @@ await Assert.ThrowsAsync<InvalidOperationException>(
6969
async () => await auth.CreateCustomTokenAsync("user"));
7070
await Assert.ThrowsAsync<InvalidOperationException>(
7171
async () => await auth.VerifyIdTokenAsync("user"));
72+
await Assert.ThrowsAsync<InvalidOperationException>(
73+
async () => await auth.SetCustomUserClaimsAsync("user", null));
7274
}
7375

7476
[Fact]

FirebaseAdmin/FirebaseAdmin/Auth/FirebaseAuth.cs

Lines changed: 44 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,8 @@ private FirebaseAuth(FirebaseApp app)
3939
() => FirebaseTokenFactory.Create(this.app), true);
4040
this.idTokenVerifier = new Lazy<FirebaseTokenVerifier>(
4141
() => FirebaseTokenVerifier.CreateIDTokenVerifier(this.app), true);
42-
this.userManager = new Lazy<FirebaseUserManager>(() =>
43-
FirebaseUserManager.Create(this.app));
42+
this.userManager = new Lazy<FirebaseUserManager>(
43+
() => FirebaseUserManager.Create(this.app), true);
4444
}
4545

4646
/// <summary>
@@ -211,17 +211,7 @@ public async Task<string> CreateCustomTokenAsync(
211211
IDictionary<string, object> developerClaims,
212212
CancellationToken cancellationToken)
213213
{
214-
FirebaseTokenFactory tokenFactory;
215-
lock (this.authLock)
216-
{
217-
if (this.deleted)
218-
{
219-
throw new InvalidOperationException("Cannot invoke after deleting the app.");
220-
}
221-
222-
tokenFactory = this.tokenFactory.Value;
223-
}
224-
214+
var tokenFactory = this.IfNotDeleted(() => this.tokenFactory.Value);
225215
return await tokenFactory.CreateCustomTokenAsync(
226216
uid, developerClaims, cancellationToken).ConfigureAwait(false);
227217
}
@@ -268,15 +258,8 @@ public async Task<FirebaseToken> VerifyIdTokenAsync(string idToken)
268258
public async Task<FirebaseToken> VerifyIdTokenAsync(
269259
string idToken, CancellationToken cancellationToken)
270260
{
271-
lock (this.authLock)
272-
{
273-
if (this.deleted)
274-
{
275-
throw new InvalidOperationException("Cannot invoke after deleting the app.");
276-
}
277-
}
278-
279-
return await this.idTokenVerifier.Value.VerifyTokenAsync(idToken, cancellationToken)
261+
var idTokenVerifier = this.IfNotDeleted(() => this.idTokenVerifier.Value);
262+
return await idTokenVerifier.VerifyTokenAsync(idToken, cancellationToken)
280263
.ConfigureAwait(false);
281264
}
282265

@@ -295,22 +278,39 @@ public async Task<FirebaseToken> VerifyIdTokenAsync(
295278
/// <param name="claims">The claims to be stored on the user account, and made
296279
/// available to Firebase security rules. These must be serializable to JSON, and after
297280
/// serialization it should not be larger than 1000 characters.</param>
298-
public async Task SetCustomUserClaimsAsync(string uid, IReadOnlyDictionary<string, object> claims)
281+
public async Task SetCustomUserClaimsAsync(
282+
string uid, IReadOnlyDictionary<string, object> claims)
299283
{
300-
lock (this.authLock)
301-
{
302-
if (this.deleted)
303-
{
304-
throw new InvalidOperationException("Cannot invoke after deleting the app.");
305-
}
306-
}
284+
await this.SetCustomUserClaimsAsync(uid, claims, default(CancellationToken));
285+
}
307286

287+
/// <summary>
288+
/// Sets the specified custom claims on an existing user account. A null claims value
289+
/// removes any claims currently set on the user account. The claims should serialize into
290+
/// a valid JSON string. The serialized claims must not be larger than 1000 characters.
291+
/// </summary>
292+
/// <returns>A task that completes when the claims have been set.</returns>
293+
/// <exception cref="ArgumentException">If <paramref name="uid"/> is null, empty or longer
294+
/// than 128 characters. Or, if the serialized <paramref name="claims"/> is larger than 1000
295+
/// characters.</exception>
296+
/// <param name="uid">The user ID string for the custom claims will be set. Must not be null
297+
/// or longer than 128 characters.
298+
/// </param>
299+
/// <param name="claims">The claims to be stored on the user account, and made
300+
/// available to Firebase security rules. These must be serializable to JSON, and after
301+
/// serialization it should not be larger than 1000 characters.</param>
302+
/// <param name="cancellationToken">A cancellation token to monitor the asynchronous
303+
/// operation.</param>
304+
public async Task SetCustomUserClaimsAsync(
305+
string uid, IReadOnlyDictionary<string, object> claims, CancellationToken cancellationToken)
306+
{
307+
var userManager = this.IfNotDeleted(() => this.userManager.Value);
308308
var user = new UserRecord(uid)
309309
{
310310
CustomClaims = claims,
311311
};
312312

313-
await this.userManager.Value.UpdateUserAsync(user);
313+
await userManager.UpdateUserAsync(user, cancellationToken).ConfigureAwait(false);
314314
}
315315

316316
/// <summary>
@@ -332,5 +332,18 @@ void IFirebaseService.Delete()
332332
}
333333
}
334334
}
335+
336+
private TResult IfNotDeleted<TResult>(Func<TResult> func)
337+
{
338+
lock (this.authLock)
339+
{
340+
if (this.deleted)
341+
{
342+
throw new InvalidOperationException("Cannot invoke after deleting the app.");
343+
}
344+
345+
return func();
346+
}
347+
}
335348
}
336349
}

FirebaseAdmin/FirebaseAdmin/Auth/FirebaseUserManager.cs

Lines changed: 40 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,11 @@
1414

1515
using System;
1616
using System.Net.Http;
17+
using System.Threading;
1718
using System.Threading.Tasks;
1819
using Google.Apis.Auth.OAuth2;
1920
using Google.Apis.Http;
21+
using Google.Apis.Json;
2022
using Newtonsoft.Json.Linq;
2123

2224
namespace FirebaseAdmin.Auth
@@ -64,22 +66,17 @@ public static FirebaseUserManager Create(FirebaseApp app)
6466
/// </summary>
6567
/// <exception cref="FirebaseException">If the server responds that cannot update the user.</exception>
6668
/// <param name="user">The user which we want to update.</param>
67-
public async Task UpdateUserAsync(UserRecord user)
69+
/// <param name="cancellationToken">A cancellation token to monitor the asynchronous
70+
/// operation.</param>
71+
public async Task UpdateUserAsync(
72+
UserRecord user, CancellationToken cancellationToken = default(CancellationToken))
6873
{
69-
var updatePath = "/accounts:update";
70-
var resopnse = await this.PostAsync(updatePath, user);
71-
72-
try
73-
{
74-
var userResponse = resopnse.ToObject<UserRecord>();
75-
if (userResponse.Uid != user.Uid)
76-
{
77-
throw new FirebaseException($"Failed to update user: {user.Uid}");
78-
}
79-
}
80-
catch (Exception e)
74+
const string updatePath = "accounts:update";
75+
var response = await this.PostAndDeserializeAsync<JObject>(
76+
updatePath, user, cancellationToken).ConfigureAwait(false);
77+
if (response["localId"]?.Value<string>() != user.Uid)
8178
{
82-
throw new FirebaseException("Error while calling Firebase Auth service", e);
79+
throw new FirebaseException($"Failed to update user: {user.Uid}");
8380
}
8481
}
8582

@@ -88,31 +85,42 @@ public void Dispose()
8885
this.httpClient.Dispose();
8986
}
9087

91-
private async Task<JObject> PostAsync(string path, UserRecord user)
88+
private async Task<TResult> PostAndDeserializeAsync<TResult>(
89+
string path, object body, CancellationToken cancellationToken)
9290
{
93-
var requestUri = $"{this.baseUrl}{path}";
94-
HttpResponseMessage response = null;
9591
try
9692
{
97-
response = await this.httpClient.PostJsonAsync(requestUri, user, default);
98-
var json = await response.Content.ReadAsStringAsync();
99-
100-
if (response.IsSuccessStatusCode)
101-
{
102-
return JObject.Parse(json);
103-
}
104-
else
105-
{
106-
var error = "Response status code does not indicate success: "
107-
+ $"{(int)response.StatusCode} ({response.StatusCode})"
108-
+ $"{Environment.NewLine}{json}";
109-
throw new FirebaseException(error);
110-
}
93+
var json = await this.PostAsync(path, body, cancellationToken)
94+
.ConfigureAwait(false);
95+
return NewtonsoftJsonSerializer.Instance.Deserialize<TResult>(json);
96+
}
97+
catch (FirebaseException)
98+
{
99+
throw;
111100
}
112-
catch (HttpRequestException e)
101+
catch (Exception e)
113102
{
114103
throw new FirebaseException("Error while calling Firebase Auth service", e);
115104
}
116105
}
106+
107+
private async Task<string> PostAsync(
108+
string path, object body, CancellationToken cancellationToken)
109+
{
110+
var requestUri = $"{this.baseUrl}/{path}";
111+
var response = await this.httpClient
112+
.PostJsonAsync(requestUri, body, cancellationToken)
113+
.ConfigureAwait(false);
114+
var json = await response.Content.ReadAsStringAsync().ConfigureAwait(false);
115+
if (!response.IsSuccessStatusCode)
116+
{
117+
var error = "Response status code does not indicate success: "
118+
+ $"{(int)response.StatusCode} ({response.StatusCode})"
119+
+ $"{Environment.NewLine}{json}";
120+
throw new FirebaseException(error);
121+
}
122+
123+
return json;
124+
}
117125
}
118126
}

0 commit comments

Comments
 (0)