Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Documentation #185

Closed
apowers313 opened this issue Jun 23, 2018 · 27 comments
Closed

Documentation #185

apowers313 opened this issue Jun 23, 2018 · 27 comments
Assignees

Comments

@apowers313
Copy link
Contributor

Moving this conversation from webauthn-open-source/fido2-lib#15

I'd like to help setup documentation for PKI.js because I think it's a great project and it has a huge amount of value that people can't get to because they can't figure out how to use it. I agree that it's a daunting amount of documentation to create, so let's not document the entire library -- let's just document the pieces that people will use right away.

Here's my opening bid:

  • There seems to be a good deal of JavaDoc-style comments sprinkled throughout the code, which can automatically be turned into documentation via JSDoc or ESDoc. ESDoc may technically be better (I like the documentation coverage feature), but I'm more familiar with JSDoc and could have a PR for that in just a few minutes. The documentation isn't going to be very comprehensive, but it will at least be easier to see what classes and methods exist.
  • I think I can (almost?) create tutorials for two classes: Certificate and CertificateChainValidationEngine. As mentioned in Incorrect function webauthn-open-source/fido2-lib#15 I may need help explaining how to use the values contained in a Certificate.

Let me know if you think that would be a worthwhile PR.

@rmhrisk
Copy link
Contributor

rmhrisk commented Jun 23, 2018

@apowers313 we have had on our long list of to-dos to add documentation for quite a long time so help in this area would certainly be appreciated.

As for starting small and expanding, this seems reasonable.

As for which style to use, I am most familiar with JSDoc as we use it on some other projects.

@YuryStrozhevsky what do you think?

Ryan

@YuryStrozhevsky
Copy link
Collaborator

@apowers313 Documentation is good, of course, but first of all let me explain basic concepts of PKIjs.

Firstly major basis of PKIjs is ASN1js: nothing would be built without ASN.1 structures. So, in order to understand PKI you would need to have at least basic understanding of ASN.1. Thus in order to understand PKIjs you would need to understand ASN1js first, at least on a basic level.

Then you need to understand that PKIjs does not make its own structures - all of them came from PKI-related RFC's. Each class from PKIjs has corresponding ASN.1 structure defined in one of RFC. Right before definition of each class you will see, for example, Class from RFC5280. Moreover, any properties defined in any class in most cases have the same names as defined in RFC. So, in order to understand what any property from any PKIjs class is you need to check related RFC - there you could find full explanation. ASN.1 definition of each class you could find also in static schema() function in each class. There you will find something like:

	static schema(parameters = {})
	{
		// Attribute { ATTRIBUTE:IOSet } ::= SEQUENCE {
		//    type   ATTRIBUTE.&id({IOSet}),
		//    values SET SIZE(1..MAX) OF ATTRIBUTE.&Type({IOSet}{@type})
		//}

At the same time you could check constructor:

	/**
	 * Constructor for Attribute class
	 * @param {Object} [parameters={}]
	 * @property {Object} [schema] asn1js parsed value
	 */
	constructor(parameters = {})
	{
		//region Internal properties of the object
		/**
		 * @type {string}
		 * @description type
		 */
		this.type = getParametersValue(parameters, "type", Attribute.defaultValues("type"));
		/**
		 * @type {Array}
		 * @description values
		 */
		this.values = getParametersValue(parameters, "values", Attribute.defaultValues("values"));
		//endregion
		
		//region If input argument array contains "schema" for this object
		if("schema" in parameters)
			this.fromSchema(parameters.schema);
		//endregion
	}

As you can see properties from ASN.1 structure are the same as properties defined in PKIjs class and types of all properties are also already defined in constructor.

Full table with all PKIjs classes and related RFCs you could find here.

So, in conclusion: documentation is good, but first of all any user of PKIjs MUST understand ASN.1 a little plus read related RFCs. After that, I bet, any user would not require any additional documentation :)

@apowers313 I would like to have your opinion about all the above as a "side person", not related directly to PKIjs project :)

@apowers313
Copy link
Contributor Author

Here is my experience in using pki.js:

The two things that attracted me to pki.js were: 1) that it was being used by a Mozilla security engineer; and 2) the claim on the home page that it "passed almost all tests from NIST PKITS". My first interaction with pkijs was "holy crap, I don't know how to use this and there's no documentation. hell no!". So I went off and tried jsrsasign and a few other PKI libraries. None of them gave me the confidence that they were going to be as comprehensive as pki.js, so I came back.

It has been a long, painful learning curve for things that should be pretty straightforward. Here is my recollection:

  1. I had to figure out how to use pki.js on nodejs. There was no documentation other than saying it was supported. I submitted a PR so that nobody would have to read through the issues and figure it out for themselves again, but even then I got the fact of whether babel-polyfill was required for nodejs wrong.
  2. Then I had to figure out how to use Certificate. Even after spending time reading through issues, test code, code, etc. I couldn't figure it out, so you can go back and look at my issue trying to figure out how to get a cert. To you it was probably a few minutes to answer; to me it was at least an hour of researching so that I wasn't asking a stupid, repetitive question. If I hadn't been committed to the project or the use of pki.js, I would have certainly walked away at that point.
  3. FIDO has a lot of unique x509 extensions, so I spent a week chasing them down, parsing them, and trying to figure out a reliable way to get values out of the valueBlocks. Do I look for hexValue? Value? Is there some reliable way to do this? I still don't know. It wasn't until much later that I noticed that pki.js had some intelligence around certain extensions and there might be yet another way of understanding the values. To this day, I'm still not sure how I'm "supposed to" get values out of certificates. I've spent plenty of time reading and understanding RFCs, but I've spent just as much time (if not more) trying to figure out the data structures of pki.js.
  4. Then, when it finally came time to validate certificate chains, I searched the code for "NIST", found the NIST test suite and read through it until I thought that I knew how to validate a cert chain. Are there corner cases that I'm not covering? Am I doing it the most simple and elegant way? I don't know.

So, yes, users need to read and understand RFCs. But pki.js is another layer on top of that world that they have to learn, and documentation would ensure that people actually use this project, have less fear around their implementations being correct, and waste less time researching things that should be part of the "pki.js 101 course".

To be fair, I get your point that this is a big project and writing documentation for all of it would take forever. But don't let the perfect be the enemy of the good. The README.md is a good starting point, there just needs to be a bit more depth and examples and I think people will get what they need -- and you can incrementally add more over time until you have documentation for the things most people care about.

One last comment about one of your statements above:

you could find full explanation. ASN.1 definition of each class

I've been using pki.js for months and I didn't know that! That would have been a super-helpful tip up front. (Is it there and I just missed it?) But also, the code is just obfuscating the documentation at that point -- trying to see all the classes and methods by reading the code is hard. All the methods aren't neatly listed on one page, and I have to scroll through pages of code, hoping that I notice where the methods are. (I'm still not sure what all the methods for Certificate are... is it just verify? And I've spent hours reading that code.) There's a reason why there are tools to parse all of this into HTML and make it user friendly.

I believe in this project and I have a great deal of sympathy for anyone that's going to try to use it without documentation, so my offer still stands to try and make things better. I'm going to need your help with CircleCI and merging docs with your current gh-pages branch. Let me know if you're interested.

@YuryStrozhevsky
Copy link
Collaborator

@apowers313 First of all your opinion do helpful and thank you very much for it! I do hope you will continue to be such open and detailed :)

But your opinion about the PKIjs scares me, frankly speaking. In fact I made PKIjs as a set of classes, directly defined the same way as in related RFC. Most methods in each class are same, before all of them there is small description. I spent a lot of time writing examples and these examples have almost the same size as PKIjs! You had no idea how to use Certificate? This example is not enough? You had no idea how to use certificate validation engine? Have you seen this example? Had no idea how to use PKIjs in Node? I wrote this example for it.

For me PKIjs made in a very trivial way. Initially we have a binary data. Then such binary data transforms to ASN.1 structures (asn1js.fromBER). Then these ASN.1 structures transoform to kind of "helpers" you are using in your code (<class>.fromSchema). Then when you need to have a binary data you got in oposite direction: transform data from "helpers" to ASN.1 (<class>.toSchema) and then ASN.1 data transform to binary representation (asn1js.toBER). Almost all classes from PKIjs are only a "helpers" for ASN.1 structures and each of such structures defined in related RFC. Nothing magic! It is nearly trivial! It is repeated in all classes from PKIjs, one-by-one!

I'm still not sure what all the methods for Certificate are... is it just verify?

Here is the list of all methods from Certificate class:

  • constructor
  • defaultValues
  • schema
  • fromSchema
  • encodeTBS
  • toSchema
  • toJSON
  • getPublicKey
  • getKeyHash
  • sign
  • verify

All these methods have description of the method and all parameters. Also bellow is the list of methods in Certificate which are specific to this class only:

  • encodeTBS
  • getPublicKey
  • getKeyHash
  • sign
  • verify

All other methods exists in ALL PKIjs classes! There are common!

Anyway, I do want to have a feedback from you and having such feedback defenitelly would help to improve PKIjs. Probably I was wrong deciding that having clear repeated methods in all classes and having huge set of example would solve all possible user's problems.

@apowers313
Copy link
Contributor Author

In fact I made PKIjs as a set of classes, directly defined the same way as in related RFC.

As someone who has read and implemented all the RFCs, that makes perfect sense. Your users probably aren't starting off with that same knowledge and experience.

You had no idea how to use Certificate? This example is not enough?

It's too much. It could be distilled down to this: #160

You had no idea how to use certificate validation engine? Have you seen this example?

That's 1.6MB of text. I eventually found what I was looking for in that file, but it certainly could have been easier.

Probably I was wrong deciding that having clear repeated methods in all classes and having huge set of example would solve all possible user's problems.

I think that's a good idea, I just didn't know that's how you had structured your code. Honestly, we should just cut and paste this thread and use it as the starting point for documentation -- this is everything I wish I knew earlier.

This could be the first paragraph of the documentation:

For me PKIjs made in a very trivial way. Initially we have a binary data. Then such binary data transforms to ASN.1 structures (asn1js.fromBER). Then these ASN.1 structures transoform to kind of "helpers" you are using in your code (.fromSchema). Then when you need to have a binary data you got in oposite direction: transform data from "helpers" to ASN.1 (.toSchema) and then ASN.1 data transform to binary representation (asn1js.toBER). Almost all classes from PKIjs are only a "helpers" for ASN.1 structures and each of such structures defined in related RFC. Nothing magic! It is nearly trivial! It is repeated in all classes from PKIjs, one-by-one!

To summarize:

  • Examples are great, but they aren't everything that users need. At different times during coding, users will need different information (syntax, key concepts, architectural overview, etc.). I found Documenting Software Architectures to be very insightful in thinking about how to explain things to people.
  • Esdoc should provide a "table of contents" for the code, letting users know what "things" exist and where to find them.
  • We should provide some tutorials for key concepts and architecture, so that people know how the pieces fit together and how they should be thinking about the code.
  • "You can find it if you look hard enough" isn't something most users will tolerate. I think it's an awesome project and more people should use it, but they're going to need help. Think about the projects you use and what documentation you've found helpful -- that's probably a good place to start.

JSDoc choked on your class structures -- I'm not sure why, but it was going to be a pain to fix (it probably required adding @Class tags to everything).

ESDoc worked much better. Here's what it looks like:
https://apowers313.github.io/pkijs-docs-test

NOTE: docs:publish may wipe out your gh-pages branch, which I see you're using for the main website. You may want to play with a test repo before using that feature, or I can write a quick script to checkout the gh-pages branch and commit the new docs.

@YuryStrozhevsky
Copy link
Collaborator

YuryStrozhevsky commented Jun 24, 2018

@apowers313 As I can see you just do not want to check exsiting examples - there are too complicated for you probably. I can not (and really do not want to see it in PKIjs) make a "different levels" of the same examples, for people with different skills level. I would better write one complex example requiring a "middle level" skills. Also you still think that it is possibility to build a useful documentation of PKIjs. On this I can say that you will end up re-writing documentation from RFC and that is all. All you need to understand is that PKIjs just provides a set of helpers to users and nothing more. Helpers for already defined structures from already known RFCs.

We should provide some tutorials for key concepts and architecture, so that people know how the pieces fit together and how they should be thinking about the code

How to explain that 2 + 2 = 4? There are set of RFCs, there are set of ASN.1 structures defined in this RFCs. ANY LIB would have the same structure: a lib would provide a helpers for such ASN.1 classes. The difference would in only properties names and "obfucation level". The PKIjs does down to a trivial: names for properties are same with names in RFC and almost all properties have "obfucation level = 0" - I am providing you exact data from ASN.1 structures wherever it possible.

I should say that you are not the first asking me for documentation - PKIjs existst for FOUR YEARS at the moment. And of course I had been thinking on a better way how to provide it. But in any cases I goes to same: if I would write a more or less complex documentation I would finish with cut-paste from RFC, nothing else. Thus in PKIjs V2 I made the README with direct links between PKIjs classes and RFCs - I can not provide anything better. Also I put a lot of efforts writing clear and complex examples. And if someone do not want to understand "the 1.6Mb of text" I do think it is not a problem with the example, but with user. Btw, as for the NIST PKITS example - the 1.6Mb is because all test certificates/CRLs are integrated in the source code. Moreover, the PKITS (PKI Test Suite) itself has a lot of tests and all of them are defined in the source code.

@apowers313
Copy link
Contributor Author

As I'm continuing to think about this, the entire thread boils down to one basic thing: time.

I use pki.js because it saves me time from writing the code myself. Likewise, documentation should save me the time of having to read the code to understand how it works. There are different forms of documentation that get me information I need. API documentation is the fastest way to understand what classes and methods are available, and their syntax. When initially learning, some overview of key concepts (e.g. - Certificate) and architecture (e.g. - "all the classes have the following common methods...") saves me the time of discovering those things for myself.

Reading dozens of RFCs and searching through code is not very time efficient.

@YuryStrozhevsky
Copy link
Collaborator

Reading dozens of RFCs and searching through code is not very time efficient

Sorry, in order to build a rocket for the fly to Mars you must know rocket science :) But if you do not want to wait learning then you could go and modify exsiting examples. For more advanced usage you would need to deeply learn, there is not other way.

@apowers313
Copy link
Contributor Author

As I can see you just do not want to check exsiting examples - there are too complicated for you probably.

Yes, but not because I'm stupid -- because it takes too much time to read large examples.

I can not (and really do not want to see it in PKIjs) make a "different levels" of the same examples, for people with different skills level.

Agreed. More examples aren't the solution.

How to explain that 2 + 2 = 4?

Yes, but not that simplistic. More like:
image

Which isn't hard if you've taken calculus (in fact, it's pretty obvious), but you might want to explain it to newbies.

you are not the first asking me for documentation

I'm not asking you for documentation. I'm offering to write it for you.

@apowers313
Copy link
Contributor Author

Sorry, in order to build a rocket for the fly to Mars you must know rocket science :) But if you do not want to wait learning then you could go and modify exsiting examples. For more advanced usage you would need to deeply learn, there is not other way.

Indeed, but the pilot of the rocket doesn't need to know the chemical composition of the fuel or the mechanical structure of the rocket. He has a convenient dashboard to use.

Of course advanced usage requires more deep learning. But not everyone requires advanced usage.

Have we reached an impasse where this isn't going to happen and I should just give up?

@YuryStrozhevsky
Copy link
Collaborator

Have we reached an impasse where this isn't going to happen and I should just give up?

I just can not uderstand what else I can provide to users! :) Again: all methods documented, all methods have primitive naming, all properties explained in RFCs, there are a table with links between class names and RFCs, number of examples are huge. What else you do need? You think the Web page with all classes would help you? Then it is trivial how to get it - just run a "doc compiler" and get it. You think it is not enough information? Where exactly? What exactly is not clear for you? Really, any specific questions about what is not clear, please.

Again, I am not against documentation, no. I really do not understand what I can give you more :)

@YuryStrozhevsky
Copy link
Collaborator

YuryStrozhevsky commented Jun 24, 2018

Indeed, but the pilot of the rocket doesn't need to know the chemical composition of the fuel or the mechanical structure of the rocket

If you just want to fly and not to build a rocket then go to existing rocket (example) and fly :)

P.S.: I am off to bed now. Will answer on my morning in case there are questions left.

@rmhrisk
Copy link
Contributor

rmhrisk commented Jun 25, 2018

Guys, I like the idea of us including the generated class documentation.

It is useful.

I also think we can provide basic inline examples via annotation.

We did this at Microsoft and it helped make calling conventions clearer.

This seems like a safe middle ground to start at doesn’t it?

@YuryStrozhevsky
Copy link
Collaborator

I made this change in documentation. Would it be enough?

@rmhrisk
Copy link
Contributor

rmhrisk commented Jun 25, 2018

That is helpful but why don’t you want a enumeration of classes like the example apowers313 provided above?

And then why not have example use cases for each major class?

@apowers313
Copy link
Contributor Author

This seems like a safe middle ground to start at doesn’t it?

I think that's useful. Maybe a link from the README.md to some of the major classes too, so that people can understand what functionality is available?

@YuryStrozhevsky
Copy link
Collaborator

There are examples for each major class with full list of possible use cases.

@rmhrisk
Copy link
Contributor

rmhrisk commented Jun 25, 2018

@YuryStrozhevsky what do you not like about having something like : https://apowers313.github.io/pkijs-docs-test/

@YuryStrozhevsky
Copy link
Collaborator

No, I am not against it. But any user could have the same already by running any doc compiler.

@rmhrisk
Copy link
Contributor

rmhrisk commented Jun 26, 2018

Normally projects provide generated docs instead of requiring users to figure out which doc generator will work with a project and produces usable documents.

Let’s provide a npm command that generate these docs and host the generated docs on the PKIjs website.

Is this a PR you could provide apowers313?

@apowers313 apowers313 mentioned this issue Jun 26, 2018
@apowers313
Copy link
Contributor Author

@rmhrisk sure, see #186

@YuryStrozhevsky
Copy link
Collaborator

I will wait on this issue to be resolved.

@YuryStrozhevsky
Copy link
Collaborator

@apowers313 Also I do recommend you to create esdoc parameter in package.json instead of making separate esdoc.json file.

@apowers313
Copy link
Contributor Author

Also I do recommend you to create esdoc parameter in package.json

Good idea, I didn't know about that feature.

@YuryStrozhevsky
Copy link
Collaborator

Updated documentation. Also I made this item especially for @apowers313 since seems he have not seen this file before and I bet most of other users also have not seen it.

@rmhrisk
Copy link
Contributor

rmhrisk commented Jun 26, 2018

Yury let’s update the PKIjs website with this new generated help resource also

@YuryStrozhevsky
Copy link
Collaborator

It is not finished yet - I will update the documentation soon. Then I will publish it on site.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants