From 4cad4e0f45111ad6ed4531c63b2677e1c26ec974 Mon Sep 17 00:00:00 2001 From: Jordan Dominion Date: Sun, 6 Oct 2024 22:18:40 -0400 Subject: [PATCH 1/4] Fix issues with login result fields being null --- build/Version.props | 2 +- src/Tgstation.Server.Api/ApiHeaders.cs | 5 ++-- .../AuthenticatedGraphQLServerClient.cs | 2 +- .../GQL/Mutations/Login.graphql | 4 ++- .../LoginResultExtensions.cs | 5 ++-- .../Authority/ILoginAuthority.cs | 4 +-- .../Authority/LoginAuthority.cs | 30 +++++++++---------- .../Controllers/ApiRootController.cs | 2 +- .../ApplicationBuilderExtensions.cs | 5 +++- src/Tgstation.Server.Host/GraphQL/Mutation.cs | 4 +-- .../{LoginPayload.cs => LoginResult.cs} | 3 +- .../Live/RawRequestTests.cs | 4 +-- 12 files changed, 38 insertions(+), 32 deletions(-) rename src/Tgstation.Server.Host/GraphQL/Mutations/Payloads/{LoginPayload.cs => LoginResult.cs} (88%) diff --git a/build/Version.props b/build/Version.props index a741ed3363b..aaa8e83481f 100644 --- a/build/Version.props +++ b/build/Version.props @@ -6,7 +6,7 @@ 6.10.0 5.3.0 10.10.0 - 0.1.0 + 0.2.0 7.0.0 16.0.0 19.0.0 diff --git a/src/Tgstation.Server.Api/ApiHeaders.cs b/src/Tgstation.Server.Api/ApiHeaders.cs index b023727beda..2f670f50296 100644 --- a/src/Tgstation.Server.Api/ApiHeaders.cs +++ b/src/Tgstation.Server.Api/ApiHeaders.cs @@ -214,7 +214,7 @@ void AddError(HeaderErrorTypes headerType, string message) var jsonAccept = new Microsoft.Net.Http.Headers.MediaTypeHeaderValue(ApplicationJsonMime); var eventStreamAccept = new Microsoft.Net.Http.Headers.MediaTypeHeaderValue(TextEventStreamMime); - if (!requestHeaders.Accept.Any(jsonAccept.IsSubsetOf)) + if (!requestHeaders.Accept.Any(accept => accept.IsSubsetOf(jsonAccept))) if (!allowEventStreamAccept) AddError(HeaderErrorTypes.Accept, $"Client does not accept {ApplicationJsonMime}!"); else if (!requestHeaders.Accept.Any(eventStreamAccept.IsSubsetOf)) @@ -358,8 +358,9 @@ void AddError(HeaderErrorTypes headerType, string message) /// /// Checks if the is compatible with . /// + /// The that can alternatively be used as the . /// if the API is compatible, otherwise. - public bool Compatible() => CheckCompatibility(ApiVersion); + public bool Compatible(Version? alternateApiVersion = null) => CheckCompatibility(ApiVersion) || (alternateApiVersion != null && alternateApiVersion.Major == ApiVersion.Major); /// /// Set using the . This initially clears . diff --git a/src/Tgstation.Server.Client.GraphQL/AuthenticatedGraphQLServerClient.cs b/src/Tgstation.Server.Client.GraphQL/AuthenticatedGraphQLServerClient.cs index 8babf9765e5..f69418d5a5a 100644 --- a/src/Tgstation.Server.Client.GraphQL/AuthenticatedGraphQLServerClient.cs +++ b/src/Tgstation.Server.Client.GraphQL/AuthenticatedGraphQLServerClient.cs @@ -70,7 +70,7 @@ public AuthenticatedGraphQLServerClient( loginResult) { this.getRestClientForToken = getRestClientForToken ?? throw new ArgumentNullException(nameof(getRestClientForToken)); - restClient = getRestClientForToken(loginResult.Data!.Login.Bearer!.EncodedToken); + restClient = getRestClientForToken(loginResult.Data!.Login.LoginResult!.Bearer.EncodedToken); } /// diff --git a/src/Tgstation.Server.Client.GraphQL/GQL/Mutations/Login.graphql b/src/Tgstation.Server.Client.GraphQL/GQL/Mutations/Login.graphql index 2d99ea65337..2fb2e9a95b7 100644 --- a/src/Tgstation.Server.Client.GraphQL/GQL/Mutations/Login.graphql +++ b/src/Tgstation.Server.Client.GraphQL/GQL/Mutations/Login.graphql @@ -1,6 +1,8 @@ mutation Login { login { - bearer + loginResult { + bearer + } errors { ... on ErrorMessageError { message diff --git a/src/Tgstation.Server.Client.GraphQL/LoginResultExtensions.cs b/src/Tgstation.Server.Client.GraphQL/LoginResultExtensions.cs index 5ec5360815d..cdfaab9d3b7 100644 --- a/src/Tgstation.Server.Client.GraphQL/LoginResultExtensions.cs +++ b/src/Tgstation.Server.Client.GraphQL/LoginResultExtensions.cs @@ -53,8 +53,7 @@ public static JsonWebToken EnsureSuccess(this IOperationResult log } } - var bearer = data.Bearer; - if (bearer == null) + if (data.LoginResult == null) { if (errors != null) { @@ -68,7 +67,7 @@ public static JsonWebToken EnsureSuccess(this IOperationResult log throw new AuthenticationException($"Null bearer and error fields!"); } - return bearer; + return data.LoginResult.Bearer; } } } diff --git a/src/Tgstation.Server.Host/Authority/ILoginAuthority.cs b/src/Tgstation.Server.Host/Authority/ILoginAuthority.cs index de0258db404..b55e94be643 100644 --- a/src/Tgstation.Server.Host/Authority/ILoginAuthority.cs +++ b/src/Tgstation.Server.Host/Authority/ILoginAuthority.cs @@ -15,7 +15,7 @@ public interface ILoginAuthority : IAuthority /// Attempt to login to the server with the current crentials. /// /// The for the operation. - /// A resulting in a and . - ValueTask> AttemptLogin(CancellationToken cancellationToken); + /// A resulting in a and . + ValueTask> AttemptLogin(CancellationToken cancellationToken); } } diff --git a/src/Tgstation.Server.Host/Authority/LoginAuthority.cs b/src/Tgstation.Server.Host/Authority/LoginAuthority.cs index 7559ac023ae..9e0602d07e1 100644 --- a/src/Tgstation.Server.Host/Authority/LoginAuthority.cs +++ b/src/Tgstation.Server.Host/Authority/LoginAuthority.cs @@ -62,8 +62,8 @@ sealed class LoginAuthority : AuthorityBase, ILoginAuthority /// Generate an for a given . /// /// The to generate a response for. - /// A new, errored . - static AuthorityResponse GenerateHeadersExceptionResponse(HeadersException headersException) + /// A new, errored . + static AuthorityResponse GenerateHeadersExceptionResponse(HeadersException headersException) => new( new ErrorMessageResponse(ErrorCode.BadHeaders) { @@ -131,14 +131,14 @@ public LoginAuthority( } /// - public async ValueTask> AttemptLogin(CancellationToken cancellationToken) + public async ValueTask> AttemptLogin(CancellationToken cancellationToken) { var headers = apiHeadersProvider.ApiHeaders; if (headers == null) return GenerateHeadersExceptionResponse(apiHeadersProvider.HeadersException!); if (headers.IsTokenAuthentication) - return BadRequest(ErrorCode.TokenWithToken); + return BadRequest(ErrorCode.TokenWithToken); var oAuthLogin = headers.OAuthProvider.HasValue; @@ -168,7 +168,7 @@ public async ValueTask> AttemptLogin(Cancellatio .GetValidator(oAuthProvider); if (validator == null) - return BadRequest(ErrorCode.OAuthProviderDisabled); + return BadRequest(ErrorCode.OAuthProviderDisabled); externalUserId = await validator .ValidateResponseCode(headers.OAuthCode!, cancellationToken); @@ -177,11 +177,11 @@ public async ValueTask> AttemptLogin(Cancellatio } catch (Octokit.RateLimitExceededException ex) { - return RateLimit(ex); + return RateLimit(ex); } if (externalUserId == null) - return Unauthorized(); + return Unauthorized(); query = query.Where( x => x.OAuthConnections!.Any( @@ -192,7 +192,7 @@ public async ValueTask> AttemptLogin(Cancellatio { var canonicalUserName = User.CanonicalizeName(headers.Username!); if (canonicalUserName == User.CanonicalizeName(User.TgsSystemUserName)) - return Unauthorized(); + return Unauthorized(); if (systemIdentity == null) query = query.Where(x => x.CanonicalName == canonicalUserName); @@ -204,7 +204,7 @@ public async ValueTask> AttemptLogin(Cancellatio // No user? You're not allowed if (user == null) - return Unauthorized(); + return Unauthorized(); // A system user may have had their name AND password changed to one in our DB... // Or a DB user was created that had the same user/pass as a system user @@ -219,7 +219,7 @@ public async ValueTask> AttemptLogin(Cancellatio { // DB User password check and update if (!isLikelyDbUser || !cryptographySuite.CheckUserPassword(user, headers.Password!)) - return Unauthorized(); + return Unauthorized(); if (user.PasswordHash != originalHash) { Logger.LogDebug("User ID {userId}'s password hash needs a refresh, updating database.", user.Id); @@ -262,11 +262,11 @@ public async ValueTask> AttemptLogin(Cancellatio if (!user.Enabled!.Value) { Logger.LogTrace("Not logging in disabled user {userId}.", user.Id); - return Forbid(); + return Forbid(); } var token = tokenFactory.CreateToken(user, oAuthLogin); - var payload = new LoginPayload + var payload = new LoginResult { Bearer = token, User = ((IApiTransformable)user).ToApi(), @@ -277,7 +277,7 @@ public async ValueTask> AttemptLogin(Cancellatio Logger.LogDebug("Successfully logged in user {userId}!", user.Id); - return new AuthorityResponse(payload); + return new AuthorityResponse(payload); } } @@ -286,9 +286,9 @@ public async ValueTask> AttemptLogin(Cancellatio /// /// The to cache. /// The the was generated for. - /// The for the successful login. + /// The for the successful login. /// A representing the running operation. - private async ValueTask CacheSystemIdentity(ISystemIdentity systemIdentity, User user, LoginPayload loginPayload) + private async ValueTask CacheSystemIdentity(ISystemIdentity systemIdentity, User user, LoginResult loginPayload) { // expire the identity slightly after the auth token in case of lag var identExpiry = loginPayload.ToApi().ParseJwt().ValidTo; diff --git a/src/Tgstation.Server.Host/Controllers/ApiRootController.cs b/src/Tgstation.Server.Host/Controllers/ApiRootController.cs index 08e499f233c..433ec5716f5 100644 --- a/src/Tgstation.Server.Host/Controllers/ApiRootController.cs +++ b/src/Tgstation.Server.Host/Controllers/ApiRootController.cs @@ -186,7 +186,7 @@ public ValueTask CreateToken(CancellationToken cancellationToken) return ValueTask.FromResult(HeadersIssue(ApiHeadersProvider.HeadersException!)); } - return loginAuthority.InvokeTransformable(this, authority => authority.AttemptLogin(cancellationToken)); + return loginAuthority.InvokeTransformable(this, authority => authority.AttemptLogin(cancellationToken)); } } } diff --git a/src/Tgstation.Server.Host/Extensions/ApplicationBuilderExtensions.cs b/src/Tgstation.Server.Host/Extensions/ApplicationBuilderExtensions.cs index 637484f5bd1..9da8c4578a2 100644 --- a/src/Tgstation.Server.Host/Extensions/ApplicationBuilderExtensions.cs +++ b/src/Tgstation.Server.Host/Extensions/ApplicationBuilderExtensions.cs @@ -14,6 +14,7 @@ using Tgstation.Server.Api.Models; using Tgstation.Server.Api.Models.Response; using Tgstation.Server.Host.Configuration; +using Tgstation.Server.Host.Properties; using Tgstation.Server.Host.System; using Tgstation.Server.Host.Utils; @@ -130,7 +131,9 @@ public static void UseApiCompatibility(this IApplicationBuilder applicationBuild applicationBuilder.Use(async (context, next) => { var apiHeadersProvider = context.RequestServices.GetRequiredService(); - if (apiHeadersProvider.ApiHeaders?.Compatible() == false) + if (apiHeadersProvider.ApiHeaders?.Compatible( + Version.Parse( + MasterVersionsAttribute.Instance.RawGraphQLVersion)) == false) { await new BadRequestObjectResult( new ErrorMessageResponse(ErrorCode.ApiMismatch)) diff --git a/src/Tgstation.Server.Host/GraphQL/Mutation.cs b/src/Tgstation.Server.Host/GraphQL/Mutation.cs index 5e6fa1d10c4..c30790791ca 100644 --- a/src/Tgstation.Server.Host/GraphQL/Mutation.cs +++ b/src/Tgstation.Server.Host/GraphQL/Mutation.cs @@ -29,13 +29,13 @@ public sealed class Mutation /// The for the operation. /// A Bearer token to be used with further communication with the server. [Error(typeof(ErrorMessageException))] - public ValueTask Login( + public ValueTask Login( [Service] IGraphQLAuthorityInvoker loginAuthority, CancellationToken cancellationToken) { ArgumentNullException.ThrowIfNull(loginAuthority); - return loginAuthority.Invoke( + return loginAuthority.Invoke( authority => authority.AttemptLogin(cancellationToken)); } } diff --git a/src/Tgstation.Server.Host/GraphQL/Mutations/Payloads/LoginPayload.cs b/src/Tgstation.Server.Host/GraphQL/Mutations/Payloads/LoginResult.cs similarity index 88% rename from src/Tgstation.Server.Host/GraphQL/Mutations/Payloads/LoginPayload.cs rename to src/Tgstation.Server.Host/GraphQL/Mutations/Payloads/LoginResult.cs index 407ebab3b7d..cf30206115b 100644 --- a/src/Tgstation.Server.Host/GraphQL/Mutations/Payloads/LoginPayload.cs +++ b/src/Tgstation.Server.Host/GraphQL/Mutations/Payloads/LoginResult.cs @@ -8,12 +8,13 @@ namespace Tgstation.Server.Host.GraphQL.Mutations.Payloads /// /// Success response for a login attempt. /// - public sealed class LoginPayload : ILegacyApiTransformable + public sealed class LoginResult : ILegacyApiTransformable { /// /// The JSON Web Token (JWT) to use as a Bearer token for accessing the server. Contains an expiry time. /// [GraphQLType] + [GraphQLNonNullType] public required string Bearer { get; init; } /// diff --git a/tests/Tgstation.Server.Tests/Live/RawRequestTests.cs b/tests/Tgstation.Server.Tests/Live/RawRequestTests.cs index 69921a4b914..447ee2f621b 100644 --- a/tests/Tgstation.Server.Tests/Live/RawRequestTests.cs +++ b/tests/Tgstation.Server.Tests/Live/RawRequestTests.cs @@ -1,4 +1,4 @@ -using Microsoft.VisualStudio.TestTools.UnitTesting; +using Microsoft.VisualStudio.TestTools.UnitTesting; using Newtonsoft.Json; using System; using System.Net; @@ -461,7 +461,7 @@ static async Task TestGraphQLLogin(IRestServerClientFactory clientFactory, IRest var result = await gqlClient.RunOperation(client => client.Login.ExecuteAsync(cancellationToken), cancellationToken); Assert.IsNotNull(result.Data); - Assert.IsNull(result.Data.Login.Bearer); + Assert.IsNull(result.Data.Login.LoginResult); Assert.IsNotNull(result.Data.Login.Errors); Assert.AreEqual(1, result.Data.Login.Errors.Count); var castResult = result.Data.Login.Errors[0] is ILogin_Login_Errors_ErrorMessageError loginError; From 8d6fb4c7057af8769e4e58de2c1723de878131e2 Mon Sep 17 00:00:00 2001 From: Jordan Dominion Date: Sun, 6 Oct 2024 22:22:08 -0400 Subject: [PATCH 2/4] Bump webpanel version --- build/WebpanelVersion.props | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build/WebpanelVersion.props b/build/WebpanelVersion.props index 6d60788f203..017492de5dd 100644 --- a/build/WebpanelVersion.props +++ b/build/WebpanelVersion.props @@ -1,6 +1,6 @@ - 6.2.0 + 6.3.0 From 64eb9bc62529e1120eb942b49a13234f63251cda Mon Sep 17 00:00:00 2001 From: Jordan Dominion Date: Sun, 6 Oct 2024 22:23:25 -0400 Subject: [PATCH 3/4] Bump API library versions --- build/Version.props | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/build/Version.props b/build/Version.props index aaa8e83481f..7f94b176fe1 100644 --- a/build/Version.props +++ b/build/Version.props @@ -8,8 +8,8 @@ 10.10.0 0.2.0 7.0.0 - 16.0.0 - 19.0.0 + 16.1.0 + 19.1.0 7.3.0 5.10.0 1.5.0 From bb8a983f291501d9643222bf4ddeb33ddb6a27f1 Mon Sep 17 00:00:00 2001 From: Jordan Dominion Date: Sun, 6 Oct 2024 22:24:05 -0400 Subject: [PATCH 4/4] Fix ensure-release CD job --- .github/workflows/ci-pipeline.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci-pipeline.yml b/.github/workflows/ci-pipeline.yml index cc4afdcb38c..41bef2a36a3 100644 --- a/.github/workflows/ci-pipeline.yml +++ b/.github/workflows/ci-pipeline.yml @@ -1908,7 +1908,7 @@ jobs: name: Ensure TGS Release is Latest GitHub Release needs: [deploy-dm, deploy-rest, deploy-gql] runs-on: ubuntu-latest - if: (!(cancelled() || failure())) && (!contains(github.event.head_commit.message, '[TGSDeploy]')) && (needs.deploy-dm.result == 'success' || needs.deploy-http.result == 'success') + if: (!(cancelled() || failure())) && (!contains(github.event.head_commit.message, '[TGSDeploy]')) && (needs.deploy-dm.result == 'success' || needs.deploy-rest.result == 'success' || needs.deploy-gql.result == 'success') steps: - name: Setup dotnet uses: actions/setup-dotnet@v4