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

Updates for the security section #168

Merged
merged 13 commits into from
Sep 17, 2024
Merged

Conversation

rsill-neo4j
Copy link
Contributor

There's more in the Security section. SoonTM

@rsill-neo4j rsill-neo4j requested a review from mjfwebb August 27, 2024 08:06
modules/ROOT/pages/security/configuration.adoc Outdated Show resolved Hide resolved
modules/ROOT/pages/security/configuration.adoc Outdated Show resolved Hide resolved
modules/ROOT/pages/security/configuration.adoc Outdated Show resolved Hide resolved
modules/ROOT/pages/security/configuration.adoc Outdated Show resolved Hide resolved
modules/ROOT/pages/security/configuration.adoc Outdated Show resolved Hide resolved

=== Decoded JWTs

// What could be added here?
Copy link
Contributor Author

Choose a reason for hiding this comment

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

^

Copy link
Contributor

Choose a reason for hiding this comment

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

A decoded JWT is passed in to the context in a similar way that an encoded JWT is, but instead of using token, we use jwt, like this where the value of the jwt is passed in by code. customImplementation should be thought of as a placeholder for whatever the programmer would actually write.

const jwt = customImplementation();

const { url } = await startStandaloneServer(server, {
    listen: { port: 4000 },
    context: async ({ req }) => ({
        jwt: jwt,
    }),
});

Using jwt instead of token in the context informs the Neo4jGraphQL library that it doesn't need to decode it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

adapted, thanks

modules/ROOT/pages/security/configuration.adoc Outdated Show resolved Hide resolved

The following code block demonstrates using Apollo Server, extracting the `Authorization` header from the request, and putting it into the appropriate context field:
To use encoded JWTs, the library must to be configured with a key to decode and verify the tokens.
Copy link
Contributor

@mjfwebb mjfwebb Aug 29, 2024

Choose a reason for hiding this comment

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

This isn't the case.

An encoded token also can be decoded via a JWKS endpoint.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

rephrased

@rsill-neo4j rsill-neo4j marked this pull request as ready for review September 5, 2024 07:48
Copy link
Contributor

@lidiazuin lidiazuin left a comment

Choose a reason for hiding this comment

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

Thanks!! Left a few suggestions. It might be worth taking a look at the new section on admonitions in our style guide so you see the options we have besides note https://development.neo4j.dev/docs/docs-style-guide/content/formatting/#_admonitions.

modules/ROOT/pages/security/authentication.adoc Outdated Show resolved Hide resolved
modules/ROOT/pages/security/authentication.adoc Outdated Show resolved Hide resolved
modules/ROOT/pages/security/authentication.adoc Outdated Show resolved Hide resolved
modules/ROOT/pages/security/authentication.adoc Outdated Show resolved Hide resolved
@@ -115,7 +195,7 @@ Additionally, if this nested location contains any `.` characters in the path, f
}
----

These characters need to be escaped:
Escape these characters:
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the way you rephrase it makes it sound like it's an option, but it may not be the case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

True. Rephrasing

@@ -5,10 +5,12 @@ auth/authorization.adoc, auth/auth-directive.adoc, auth/subscriptions.adoc, \
auth/authorization/allow.adoc, auth/authorization/bind.adoc, auth/authorization/roles.adoc, \
auth/authorization/where.adoc, authentication-and-authorization/index.adoc

The Neo4j GraphQL Library offers the following security features:
Copy link
Contributor

Choose a reason for hiding this comment

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

Usually the index pages use a list with the name of the page and then a description, not incorporating the title within the description. It's not exactly a rule, but it's kinda consistent throughout docsets.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh.
well, this was my attempt to do something else with it, but it ended up pretty close to the original.
not a fan of these pages, since there's also the left side nav, which almost does the same, so the page doesn't serve much of a purpose.

but if it's a convention, then let's stick to it

modules/ROOT/pages/security/operations.adoc Outdated Show resolved Hide resolved
modules/ROOT/pages/security/operations.adoc Show resolved Hide resolved
modules/ROOT/pages/security/operations.adoc Outdated Show resolved Hide resolved
@neo-technology-commit-status-publisher
Copy link
Collaborator

Thanks for the documentation updates.

The preview documentation has now been torn down - reopening this PR will republish it.

@rsill-neo4j rsill-neo4j merged commit e9f3be9 into 5.x Sep 17, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants