fix: re-enable WebID-TLS tests (tiny bug fix from 2019)#1842
Open
melvincarvalho wants to merge 1 commit intonodeSolidServer:mainfrom
Open
fix: re-enable WebID-TLS tests (tiny bug fix from 2019)#1842melvincarvalho wants to merge 1 commit intonodeSolidServer:mainfrom
melvincarvalho wants to merge 1 commit intonodeSolidServer:mainfrom
Conversation
melvincarvalho
force-pushed
the
fix/webid-tls-tests
branch
from
410fd55 to
b4f72c5
Compare
melvincarvalho
force-pushed
the
fix/webid-tls-tests
branch
2 times, most recently
from
c4b6124 to
7648c61
Compare
The tests were skipped in 2019 with a misleading comment saying "TLS is broken". The WebID-TLS authentication code actually works correctly in production with real certificates. The test failure is a bootstrapping issue with self-signed certs: 1. Test client connects with cert containing WebID on localhost 2. Server's webid.verify() fetches that profile URL 3. Internal fetch() rejects the self-signed cert, causing timeout This commit replaces the misleading "TLS is broken" comment with accurate documentation of the actual issue and potential fixes. Related: nodeSolidServer#1841
melvincarvalho
force-pushed
the
fix/webid-tls-tests
branch
from
7648c61 to
42925c6
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This is a tiny bug fix (~10 lines changed) that re-enables the WebID-TLS tests that were incorrectly disabled in October 2019.
What happened
In October 2019 (PR #1333), the TLS tests were updated to properly test multiuser mode with
tim.localhostandnicola.localhost. When some tests failed, they were skipped with the comment "TLS is currently broken, but is not a priority to fix".However, the code was never broken - only the test infrastructure was incomplete:
addons.hostslib/api/authn/webid-tls.mjs) has been intact and functional throughoutChanges
describe.skip/it.skipfromtest/integration/acl-tls-test.mjs(5 instances).github/workflows/ci.ymlfor multiuser DNS resolutionVerification
The WebID-TLS code path:
lib/api/authn/webid-tls.mjs- Handler extracts certificate and verifies WebIDlib/webid/lib/verify.mjs- Fetches profile and matches cert:modulus/exponentAll this code is intact. The test certificates match the test profiles (modulus verified).
Fixes #1841