GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Fix tls tests by jaxoncreed · Pull Request #1333 · nodeSolidServer/node-solid-server

Conversation

@jaxoncreed

@jaxoncreed jaxoncreed commented Oct 29, 2019

Copy link
Copy Markdown
Contributor

The tests for WebID TLS didn't actually test NSS. I have updated the tests properly which show that TLS is broken on NSS. Yet, because TLS will soon be depricated I will not fix.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
jaxoncreed added 5 commits October 24, 2019 13:38
jaxoncreed
Fix TLS Tests
1a92a91
jaxoncreed
Set up file structure for tls tests
53c063b
jaxoncreed
TLS test has parsable webID
c0bea97
jaxoncreed
Skipped tls tests
778095a
jaxoncreed
Clean up
63e47bb
@jaxoncreed jaxoncreed changed the title [WIP] Fix tls tests Fix tls tests Oct 29, 2019
jaxoncreed
Code cleanup
86a4487
michielbdejong
michielbdejong approved these changes Nov 1, 2019
View reviewed changes
Comment thread test/integration/acl-tls-test.js Outdated
}

describe('ACL with WebID+TLS', function () {
// TODO Remove skip. TLS is currently broken, but is not a priority to fix

@michielbdejong michielbdejong Nov 1, 2019

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe add:
... since the current Solid spec does not require supporting webid-tls on the resource server. The current spec only requires the resource server to support webid-oidc, and it requires the IDP to support webid-tls as a log in method, so that users of a webid-tls client certificate can still use their certificate (and not a username/password pair or other login method) to "bridge" from webid-tls to webid-oidc.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@michielbdejong michielbdejong Nov 1, 2019

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or something to that extent. Would be even better if you link to a spec issue where this is stated, will post here if i find one.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@michielbdejong michielbdejong Nov 1, 2019

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, found it: solid/solid-spec#171 (comment)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@michielbdejong

michielbdejong commented Nov 1, 2019

Copy link
Copy Markdown
Member

Impressive work, Jackson! Sorry you did all this work to restore the tests for webid-tls on the resource server, only to find out that it's broken and wont-fix, but it's definitely good to have these tests working, even if they are currently skipped.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
jaxoncreed
Clarified why tests are skipped
b183c7a
@jaxoncreed
jaxoncreed merged commit e117cfc into master Nov 1, 2019
@melvincarvalho melvincarvalho mentioned this pull request Jan 8, 2026
fix: re-enable WebID-TLS tests (tiny bug fix from 2019) #1842
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@michielbdejong michielbdejong michielbdejong approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

2 participants

@jaxoncreed @michielbdejong
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.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.