From be6fe8fa88a8452ec2d52cb3042519e664ec40cf Mon Sep 17 00:00:00 2001 From: Jordan Dominion Date: Fri, 3 Nov 2023 00:34:37 -0400 Subject: [PATCH 1/5] Run systemd as the user `tgstation-server` - Fix .deb build only working in gh-actions tailored environment. - Fix indentation in `rules`. - Fix build script not working fully without a signing key. - Standardize to node 20 as a build dependency. - Fix my ADHD in the `README.md`. Closes #1658 --- .github/CONTRIBUTING.md | 2 +- README.md | 4 +++- build/package/deb/build_package.sh | 20 ++++++++++++++----- build/package/deb/debian/control | 2 +- build/package/deb/debian/postinst | 7 ++++++- build/package/deb/debian/rules | 32 ++++++++++++++++++++---------- build/package/deb/tgs-configure | 4 +--- build/tgstation-server.service | 1 + 8 files changed, 49 insertions(+), 23 deletions(-) diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index 941aee5575..b8709f641f 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -34,7 +34,7 @@ You can of course, as always, ask for help at [#coderbus](irc://irc.rizon.net/co ### Development Environment -You need the .NET 8.0 SDK and npm>=v5.7 (in your PATH) to compile the server. +You need the .NET 8.0 SDK, node>=v20, and npm>=v5.7 (in your PATH) to compile the server. The recommended IDE is Visual Studio 2022 or VSCode. diff --git a/README.md b/README.md index a288af1e99..26290b06a1 100644 --- a/README.md +++ b/README.md @@ -117,6 +117,8 @@ sudo dpkg --add-architecture i386 \ && sudo systemctl start tgstation-server ``` +The service will execute as the newly created user: `tgstation-server`. + ##### Debian The `aspnetcore-runtime-8.0` package isn't yet available on mainline Debian and must be [installed from Microsoft](https://learn.microsoft.com/en-us/dotnet/core/install/linux-debian) first. Use the following one-liner to add their packages repository. @@ -143,7 +145,7 @@ The following dependencies are required. [Download the latest release .zip](https://github.com/tgstation/tgstation-server/releases/latest). Choose `ServerConsole`. -If you have SystemD installed, we recommend installing the service unit [here](./build/tgstation-server.service). It assumes TGS is installed into `/opt/tgstation-server` and you will be using the but feel free to adjust it to your needs. Note that the server will need to have it's configuration file setup before running with SystemD. +If you have SystemD installed, we recommend installing the service unit [here](./build/tgstation-server.service). It assumes TGS is installed into `/opt/tgstation-server`, it is executing as the user `tgstation-server`, and you will be using the console runner, but feel free to adjust it to your needs. Note that the server will need to have it's configuration file setup before running with SystemD. Alternatively, to launch the server in the current shell, run `./tgs.sh` in the root of the installation directory. The process will run in a blocking fashion. SIGQUIT will close the server, terminating all live game instances. diff --git a/build/package/deb/build_package.sh b/build/package/deb/build_package.sh index 14dc1a50ed..79f2e57244 100755 --- a/build/package/deb/build_package.sh +++ b/build/package/deb/build_package.sh @@ -9,16 +9,27 @@ set -x dpkg --add-architecture i386 apt-get update # This package set needs cleanup probably, StackOverflow copypasta -apt-get install -y npm \ +apt-get install -y \ build-essential \ binutils \ lintian \ debhelper \ dh-make \ devscripts \ + ca-certificates \ + curl \ + gnupg \ xmlstarlet # dotnet-sdk-8.0 # Disabled while in preview +# https://github.com/nodesource/distributions +mkdir -p /etc/apt/keyrings +curl -fsSL https://deb.nodesource.com/gpgkey/nodesource-repo.gpg.key | gpg --dearmor -o /etc/apt/keyrings/nodesource.gpg +export NODE_MAJOR=20 +echo "deb [signed-by=/etc/apt/keyrings/nodesource.gpg] https://deb.nodesource.com/node_$NODE_MAJOR.x nodistro main" | tee /etc/apt/sources.list.d/nodesource.list +apt-get update +apt-get install nodejs -y + CURRENT_COMMIT=$(git rev-parse HEAD) rm -rf packaging @@ -56,14 +67,13 @@ set +e if [[ -z "$PACKAGING_KEYGRIP" ]]; then dpkg-buildpackage --no-sign + EXIT_CODE=$? else dpkg-buildpackage --sign-key=$PACKAGING_KEYGRIP --sign-command="$SIGN_COMMAND" + cat /tmp/tgs_wrap_gpg_output.log + EXIT_CODE=$? fi -EXIT_CODE=$? - -cat /tmp/tgs_wrap_gpg_output.log - cd .. exit $EXIT_CODE diff --git a/build/package/deb/debian/control b/build/package/deb/debian/control index 486be1bcf2..a63d1d0fab 100644 --- a/build/package/deb/debian/control +++ b/build/package/deb/debian/control @@ -5,7 +5,7 @@ Maintainer: Jordan Dominion Rules-Requires-Root: no Build-Depends: debhelper-compat (= 13), - npm, + nodejs, #dotnet-sdk-8.0, Disabled while in preview Standards-Version: 4.6.2 Homepage: https://tgstation.github.io/tgstation-server diff --git a/build/package/deb/debian/postinst b/build/package/deb/debian/postinst index 9f71a1931f..9a1cb1b13c 100755 --- a/build/package/deb/debian/postinst +++ b/build/package/deb/debian/postinst @@ -1,9 +1,14 @@ #!/bin/sh -e +adduser --system tgstation-server +mkdir -m 754 -p /var/log/tgstation-server +chown -R tgstation-server /etc/tgstation-server +chown -R tgstation-server /opt/tgstation-server +chown -R tgstation-server /var/log/tgstation-server + #DEBHELPER# if [ "$1" = "configure" ]; then - chmod 600 /etc/tgstation-server deb-systemd-helper stop 'tgstation-server.service' >/dev/null || true echo " _ _ _ _ " diff --git a/build/package/deb/debian/rules b/build/package/deb/debian/rules index a68a2cd7d7..7a13e96dd2 100755 --- a/build/package/deb/debian/rules +++ b/build/package/deb/debian/rules @@ -3,23 +3,23 @@ export DH_VERBOSE = 1 %: - dh $@ + dh $@ override_dh_auto_clean: - rm -rf artifacts - dotnet clean -c ReleaseNoWindows + rm -rf artifacts + dotnet clean -c ReleaseNoWindows override_dh_auto_build: - dotnet restore - cd src/Tgstation.Server.Host.Console && dotnet publish -c Release -o ../../artifacts - cd src/Tgstation.Server.Host && dotnet publish -c Release -o ../../artifacts/lib/Default - rm artifacts/lib/Default/appsettings.yml - build/RemoveUnsupportedRuntimes.sh artifacts/lib/Default - build/RemoveUnsupportedRuntimes.sh artifacts + dotnet restore + cd src/Tgstation.Server.Host.Console && dotnet publish -c Release -o ../../artifacts + cd src/Tgstation.Server.Host && dotnet publish -c Release -o ../../artifacts/lib/Default + rm artifacts/lib/Default/appsettings.yml + build/RemoveUnsupportedRuntimes.sh artifacts/lib/Default + build/RemoveUnsupportedRuntimes.sh artifacts override_dh_auto_install: - cp build/package/deb/MakeInstall ./Makefile - dh_auto_install -- + cp build/package/deb/MakeInstall ./Makefile + dh_auto_install -- override_dh_strip: @@ -27,3 +27,13 @@ override_dh_shlibdeps: override_dh_installsystemd: dh_installsystemd -v --restart-after-upgrade + +override_dh_fixperms: + dh_fixperms + find debian/tgstation-server/opt/tgstation-server -exec chmod 544 {} + + find debian/tgstation-server/opt/tgstation-server -type d -exec chmod 555 {} + + find debian/tgstation-server/opt/tgstation-server/lib -exec chmod 744 {} + + find debian/tgstation-server/opt/tgstation-server/lib -type d -exec chmod 755 {} + + find debian/tgstation-server/etc/tgstation-server -exec chmod 644 {} + + find debian/tgstation-server/etc/tgstation-server -type d -exec chmod 755 {} + + chmod 640 debian/tgstation-server/etc/tgstation-server/appsettings.Production.yml diff --git a/build/package/deb/tgs-configure b/build/package/deb/tgs-configure index 96186f68b0..a4bfd3e5f4 100755 --- a/build/package/deb/tgs-configure +++ b/build/package/deb/tgs-configure @@ -1,5 +1,3 @@ #!/bin/sh -cd /opt/tgstation-server -export General__SetupWizardMode=Only -exec /usr/bin/dotnet /opt/tgstation-server/lib/Default/Tgstation.Server.Host.dll /tmp/tgs_temp_should_not_be_used --appsettings-base-path=/etc/tgstation-server +exec su -s /bin/sh -c "cd /opt/tgstation-server && export General__SetupWizardMode=Only && exec /usr/bin/dotnet /opt/tgstation-server/lib/Default/Tgstation.Server.Host.dll /tmp/tgs_temp_should_not_be_used --appsettings-base-path=/etc/tgstation-server" tgstation-server diff --git a/build/tgstation-server.service b/build/tgstation-server.service index 83e8e47793..6f3c087584 100644 --- a/build/tgstation-server.service +++ b/build/tgstation-server.service @@ -7,6 +7,7 @@ After=postgresql.service After=mssql-server.service [Service] +User=tgstation-server Type=notify-reload NotifyAccess=all WorkingDirectory=/opt/tgstation-server From 90d41cc88d13a1e011eb0aa1df97f5edf4f7356c Mon Sep 17 00:00:00 2001 From: Jordan Dominion Date: Mon, 6 Nov 2023 17:56:03 -0500 Subject: [PATCH 2/5] Replace HTTP 426 with 400 Notable exception is the swarm API, which still uses 426 for version mismatches and would be a pain in the ass to change correctly. Closes #1693 --- docs/API.dox | 1 - src/Tgstation.Server.Client/ApiClient.cs | 5 +++-- src/Tgstation.Server.Host/Controllers/README.md | 1 - .../Extensions/ApplicationBuilderExtensions.cs | 5 +---- src/Tgstation.Server.Host/Security/README.md | 4 ++-- tests/Tgstation.Server.Tests/Live/RawRequestTests.cs | 4 ++-- 6 files changed, 8 insertions(+), 12 deletions(-) diff --git a/docs/API.dox b/docs/API.dox index a859e36c47..be71673604 100644 --- a/docs/API.dox +++ b/docs/API.dox @@ -72,7 +72,6 @@ TGS will only every return the response codes listed here - 410: Gone. Attempted to access/modify a resource that ideally should have been ready, but isn't or no longer is - 422: Unprocessable Entity: Used specifically when an operation that requires a server restart is unable to be performed due to the @ref Tgstation.Server.Host.Watchdog not being present in the deployment. Should not happen with a proper server configuration. Response body contains an @ref Tgstation.Server.Api.Models.ErrorMessage - 424: Failed Dependency: When a request that depends on an external API fails for a reason other than rate limiting. The response body will contain an @ref Tgstation.Server.Api.Models.ErrorMessage model detailing the error. -- 426: Upgrade required: Used when the client's API version is not compatible with the server's. Response body contains an @ref Tgstation.Server.Api.Models.ErrorMessage - 429: Rate limited. Used with operations that rely on GitHub.com. If a rate limit is hit for an operation this will be returned. Response will contain a Retry-After header with the amount of seconds to wait. - 500: Server error. Please report the request and response body to the code repository - 501: Not implemented. Functionality not available in the current server version diff --git a/src/Tgstation.Server.Client/ApiClient.cs b/src/Tgstation.Server.Client/ApiClient.cs index 28c5611bdd..3270be8ea2 100644 --- a/src/Tgstation.Server.Client/ApiClient.cs +++ b/src/Tgstation.Server.Client/ApiClient.cs @@ -131,8 +131,6 @@ namespace Tgstation.Server.Client #pragma warning restore IDE0066 // Convert switch statement to expression #pragma warning restore IDE0010 // Add missing cases { - case HttpStatusCode.UpgradeRequired: - throw new VersionMismatchException(errorMessage, response); case HttpStatusCode.Unauthorized: throw new UnauthorizedException(errorMessage, response); case HttpStatusCode.InternalServerError: @@ -154,6 +152,9 @@ namespace Tgstation.Server.Client case (HttpStatusCode)429: throw new RateLimitException(errorMessage, response); default: + if (errorMessage?.ErrorCode == ErrorCode.ApiMismatch) + throw new VersionMismatchException(errorMessage, response); + throw new ApiConflictException(errorMessage, response); } } diff --git a/src/Tgstation.Server.Host/Controllers/README.md b/src/Tgstation.Server.Host/Controllers/README.md index e1214f91f2..66c5df1244 100644 --- a/src/Tgstation.Server.Host/Controllers/README.md +++ b/src/Tgstation.Server.Host/Controllers/README.md @@ -7,7 +7,6 @@ Some notable exceptions: - [ApiController](./ApiController.cs) is the base class of nearly all API related controllers. It does the following: - Contains code to deny the request if the instance is not present when it should be. - Contains the `IDatabaseContext` and `ILogger` properties for child controllers. - - Returns 426 Upgrade Required if the API version in the headers are incompatible with the request. - Returns 400 Bad Request if the headers or the PUT/POST'd model is invalid. - Returns 401 If an `IAuthenticationContext` could not be created for a request. - [BridgeController](./BridgeController.cs) is a special controller accessible only from localhost and is used to receive bridge request from DreamDaemon diff --git a/src/Tgstation.Server.Host/Extensions/ApplicationBuilderExtensions.cs b/src/Tgstation.Server.Host/Extensions/ApplicationBuilderExtensions.cs index 55086228dc..40661c9a5e 100644 --- a/src/Tgstation.Server.Host/Extensions/ApplicationBuilderExtensions.cs +++ b/src/Tgstation.Server.Host/Extensions/ApplicationBuilderExtensions.cs @@ -132,11 +132,8 @@ namespace Tgstation.Server.Host.Extensions var apiHeadersProvider = context.RequestServices.GetRequiredService(); if (apiHeadersProvider.ApiHeaders?.Compatible() == false) { - await new JsonResult( + await new BadRequestObjectResult( new ErrorMessageResponse(ErrorCode.ApiMismatch)) - { - StatusCode = (int)HttpStatusCode.UpgradeRequired, - } .ExecuteResultAsync(new ActionContext { HttpContext = context, diff --git a/src/Tgstation.Server.Host/Security/README.md b/src/Tgstation.Server.Host/Security/README.md index 404b27e162..ac6cbbc08f 100644 --- a/src/Tgstation.Server.Host/Security/README.md +++ b/src/Tgstation.Server.Host/Security/README.md @@ -16,7 +16,7 @@ ## For the login request (`POST /`) -1. An attempt to parse the `ApiHeaders` is made. If they were valid, the API version check is performed. If it fails, HTTP 426 with an `ErrorMessageResponse` will be returned. +1. An attempt to parse the `ApiHeaders` is made. If they were valid, the API version check is performed. If it fails, HTTP 400 with an `ErrorMessageResponse` will be returned. 1. If, for some reason, the user attempts to use a JWT to authenticate this request, steps 2-4 of the non-login pipeline list below are performed. 1. The `ApiController` base class inspects the request. - At this point, if the `ApiHeaders` (MINUS the `Authorization` header) cannot be properly parsed, HTTP 400 with an `ErrorMessageResponse` is returned. @@ -51,7 +51,7 @@ ## For all other authenticated requests -1. An attempt to parse the `ApiHeaders` is made. If they were valid. The API version check is performed. If it fails, HTTP 426 with an `ErrorMessageResponse` will be returned. +1. An attempt to parse the `ApiHeaders` is made. If they were valid. The API version check is performed. If it fails, HTTP 400 with an `ErrorMessageResponse` will be returned. 1. The JWT, if present, is validated. If it is, the scope's [AuthenticationContextFactory](./AuthenticationContextFactory.cs) has `SetTokenNbf` called. If not, HTTP 401 will be returned. - Inside ASP.NET Core, this initializes the calling user's identity principal and sets the "sub" claim to the TGS user ID parsed out of the JWT. - We know it's the user ID because we set it up like that in the [TokenFactory](./TokenFactory.cs) diff --git a/tests/Tgstation.Server.Tests/Live/RawRequestTests.cs b/tests/Tgstation.Server.Tests/Live/RawRequestTests.cs index fbf5fe44e5..bf0d409e61 100644 --- a/tests/Tgstation.Server.Tests/Live/RawRequestTests.cs +++ b/tests/Tgstation.Server.Tests/Live/RawRequestTests.cs @@ -87,7 +87,7 @@ namespace Tgstation.Server.Tests.Live request.Headers.Add(ApiHeaders.ApiVersionHeader, "Tgstation.Server.Api/6.0.0"); request.Headers.Authorization = new AuthenticationHeaderValue(ApiHeaders.BearerAuthenticationScheme, token); using var response = await httpClient.SendAsync(request, cancellationToken); - Assert.AreEqual(HttpStatusCode.UpgradeRequired, response.StatusCode); + Assert.AreEqual(HttpStatusCode.BadRequest, response.StatusCode); var content = await response.Content.ReadAsStringAsync(cancellationToken); var message = JsonConvert.DeserializeObject(content); Assert.AreEqual(ErrorCode.ApiMismatch, message.ErrorCode); @@ -101,7 +101,7 @@ namespace Tgstation.Server.Tests.Live request.Headers.Add(ApiHeaders.ApiVersionHeader, "Tgstation.Server.Api/6.0.0"); request.Headers.Authorization = new AuthenticationHeaderValue(ApiHeaders.BearerAuthenticationScheme, token); using var response = await httpClient.SendAsync(request, cancellationToken); - Assert.AreEqual(HttpStatusCode.UpgradeRequired, response.StatusCode); + Assert.AreEqual(HttpStatusCode.BadRequest, response.StatusCode); var content = await response.Content.ReadAsStringAsync(cancellationToken); var message = JsonConvert.DeserializeObject(content); Assert.AreEqual(ErrorCode.ApiMismatch, message.ErrorCode); From 7c9a1a6a4fb9f04d22cbef627937278dd1b7aef7 Mon Sep 17 00:00:00 2001 From: Jordan Dominion Date: Mon, 6 Nov 2023 21:45:45 -0500 Subject: [PATCH 3/5] Fix a cause of spurious CI failures --- .../Live/Instance/JobsHubTests.cs | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/tests/Tgstation.Server.Tests/Live/Instance/JobsHubTests.cs b/tests/Tgstation.Server.Tests/Live/Instance/JobsHubTests.cs index bf683503c1..a52e6e1b1d 100644 --- a/tests/Tgstation.Server.Tests/Live/Instance/JobsHubTests.cs +++ b/tests/Tgstation.Server.Tests/Live/Instance/JobsHubTests.cs @@ -158,19 +158,24 @@ namespace Tgstation.Server.Tests.Live.Instance var seenThisJob = seenJobs.TryGetValue(job.Id.Value, out var hubJob); if (seenThisJob) { - Assert.AreEqual(job.StoppedAt, hubJob.StoppedAt); + if (hubJob.StoppedAt.HasValue) + { + Assert.AreEqual(job.StoppedAt, hubJob.StoppedAt); + Assert.AreEqual(job.ExceptionDetails, hubJob.ExceptionDetails); + Assert.AreEqual(job.Progress, hubJob.Progress); + Assert.AreEqual(job.Stage, hubJob.Stage); + Assert.AreEqual(job.ErrorCode, hubJob.ErrorCode); + Assert.AreEqual(job.CancelledBy?.Id, hubJob.CancelledBy?.Id); + Assert.AreEqual(job.Cancelled, hubJob.Cancelled); + } + Assert.AreEqual(job.InstanceId, hubJob.InstanceId); - Assert.AreEqual(job.ExceptionDetails, hubJob.ExceptionDetails); - Assert.AreEqual(job.Stage, hubJob.Stage); - Assert.AreEqual(job.CancelledBy?.Id, hubJob.CancelledBy?.Id); - Assert.AreEqual(job.Cancelled, hubJob.Cancelled); Assert.AreEqual(job.StartedBy?.Id, hubJob.StartedBy?.Id); Assert.AreEqual(job.CancelRight, hubJob.CancelRight); Assert.AreEqual(job.CancelRightsType, hubJob.CancelRightsType); - Assert.AreEqual(job.Progress, hubJob.Progress); Assert.AreEqual(job.Description, hubJob.Description); - Assert.AreEqual(job.ErrorCode, hubJob.ErrorCode); Assert.AreEqual(job.StartedAt, hubJob.StartedAt); + Assert.AreEqual(job.JobCode, hubJob.JobCode); } else { From 2f9fb5df0c66a20960c3472c2dcb45bdbc2980c6 Mon Sep 17 00:00:00 2001 From: Jordan Dominion Date: Tue, 7 Nov 2023 08:52:01 -0500 Subject: [PATCH 4/5] Suppress the NU5104 while we're in preview Added to #1589 --- src/Tgstation.Server.Client/Tgstation.Server.Client.csproj | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Tgstation.Server.Client/Tgstation.Server.Client.csproj b/src/Tgstation.Server.Client/Tgstation.Server.Client.csproj index 375fb7161a..c946acf841 100644 --- a/src/Tgstation.Server.Client/Tgstation.Server.Client.csproj +++ b/src/Tgstation.Server.Client/Tgstation.Server.Client.csproj @@ -7,6 +7,7 @@ Client library for tgstation-server. json web api tgstation-server tgstation ss13 byond client http $(TGS_NUGET_RELEASE_NOTES_CLIENT) + NU5104 From 469b16d815ec1804229a58a2fd348cf959045085 Mon Sep 17 00:00:00 2001 From: Jordan Dominion Date: Thu, 9 Nov 2023 08:53:29 -0500 Subject: [PATCH 5/5] Fix test errors caused by DB truncation --- tests/Tgstation.Server.Tests/Live/Instance/JobsHubTests.cs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/Tgstation.Server.Tests/Live/Instance/JobsHubTests.cs b/tests/Tgstation.Server.Tests/Live/Instance/JobsHubTests.cs index a52e6e1b1d..d097aae7eb 100644 --- a/tests/Tgstation.Server.Tests/Live/Instance/JobsHubTests.cs +++ b/tests/Tgstation.Server.Tests/Live/Instance/JobsHubTests.cs @@ -169,12 +169,17 @@ namespace Tgstation.Server.Tests.Live.Instance Assert.AreEqual(job.Cancelled, hubJob.Cancelled); } + static DateTimeOffset PerformDBTruncation(DateTimeOffset original) + => new DateTimeOffset( + original.Ticks - (original.Ticks % TimeSpan.TicksPerSecond), + original.Offset); + Assert.AreEqual(job.InstanceId, hubJob.InstanceId); Assert.AreEqual(job.StartedBy?.Id, hubJob.StartedBy?.Id); Assert.AreEqual(job.CancelRight, hubJob.CancelRight); Assert.AreEqual(job.CancelRightsType, hubJob.CancelRightsType); Assert.AreEqual(job.Description, hubJob.Description); - Assert.AreEqual(job.StartedAt, hubJob.StartedAt); + Assert.AreEqual(PerformDBTruncation(job.StartedAt.Value), PerformDBTruncation(hubJob.StartedAt.Value)); // RHS may NOT be DB truncated, both sides because not all DBs do this Assert.AreEqual(job.JobCode, hubJob.JobCode); } else