Skip to content

Remove insecure clockSkew recommendation - #151

Merged
markstos merged 2 commits into
node-saml:masterfrom
cjbarth:insecure-recomendation
Aug 24, 2022
Merged

Remove insecure clockSkew recommendation#151
markstos merged 2 commits into
node-saml:masterfrom
cjbarth:insecure-recomendation

Conversation

@cjbarth

@cjbarth cjbarth commented Aug 16, 2022

Copy link
Copy Markdown
Collaborator

Based on this discussion, it seems we are needlessly recommending an insecure configuration and such a recommendation is being copied by others. This removes the insecure recommendation.

@cjbarth
cjbarth requested a review from markstos August 16, 2022 01:28
@codecov

codecov Bot commented Aug 16, 2022

Copy link
Copy Markdown

Codecov Report

Merging #151 (ff59d36) into master (cf9d3bc) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #151   +/-   ##
=======================================
  Coverage   80.61%   80.61%           
=======================================
  Files          11       11           
  Lines         810      810           
  Branches      244      244           
=======================================
  Hits          653      653           
  Misses         71       71           
  Partials       86       86           
Impacted Files Coverage Δ
src/metadata.ts 90.47% <0.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@cjbarth cjbarth added the documentation Improvements or additions to documentation label Aug 16, 2022
@markstos

Copy link
Copy Markdown

The ADFS document was very old-- it pre-dates my involvement and I am not familiar with ADFS personally. I'm fine with removing this. If someone who uses passport-saml and ADFS wants to maintain their own documentation, they can. This doesn't need to be in the core project repo.

@markstos
markstos merged commit 3378c91 into node-saml:master Aug 24, 2022
@srd90

srd90 commented Aug 24, 2022

Copy link
Copy Markdown

node-saml/node-saml's main README.md has still (after this PR was merged) link to removed docs/adfs/README.md:

For more detailed instructions, see ADFS documentation.

source: README.md#usage-with-active-directory-federation-services @ 3378c915

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants