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 for edc.transfer.proxy.token... config is wrong #3986

Closed
florianrusch-zf opened this issue Mar 11, 2024 · 4 comments · Fixed by #4084
Closed

Documentation for edc.transfer.proxy.token... config is wrong #3986

florianrusch-zf opened this issue Mar 11, 2024 · 4 comments · Fixed by #4084
Assignees
Labels
documentation Improvements or additions to documentation good first issue Good for newcomers

Comments

@florianrusch-zf
Copy link
Contributor

The documentation at the following lines is wrong. There is no mechanism in place which will create random keys.

| `edc.transfer.proxy.token.signer.privatekey.alias` | Alias of private key used for signing tokens | false | Random EC public key |
| `edc.transfer.proxy.token.verifier.publickey.alias` | Alias of public key used for verifying the tokens | false | Random EC private key |

Proof:

if (publicKeyAlias != null && privateKeyAlias != null) {
var controller = new ConsumerPullTransferTokenValidationApiController(tokenValidationService, dataEncrypter, typeManager, (i) -> publicKeyService.resolveKey(publicKeyAlias));
webService.registerResource(controlApiConfiguration.getContextAlias(), controller);
var resolver = new ConsumerPullDataPlaneProxyResolver(dataEncrypter, typeManager, new JwtGenerationService(), getPrivateKeySupplier(context, privateKeyAlias), () -> publicKeyAlias, tokenExpirationDateFunction);
dataFlowManager.register(new ConsumerPullTransferDataFlowController(selectorService, resolver));
} else {
context.getMonitor().info("One of these settings is not configured, so the connector won't be able to provide 'consumer-pull' transfers: [%s, %s]"
.formatted(TOKEN_VERIFIER_PUBLIC_KEY_ALIAS, TOKEN_SIGNER_PRIVATE_KEY_ALIAS));
}

@github-actions github-actions bot added the triage all new issues awaiting classification label Mar 11, 2024
@florianrusch-zf florianrusch-zf added documentation Improvements or additions to documentation good first issue Good for newcomers and removed triage all new issues awaiting classification labels Mar 11, 2024
@florianrusch-zf florianrusch-zf changed the title Documentation for edc.transfer.proxy.token config is wrong Documentation for edc.transfer.proxy.token... config is wrong Mar 11, 2024
@ndr-brt
Copy link
Member

ndr-brt commented Mar 11, 2024

documenting the settings in a readme is highly discouraged because they get outdated easily, better to properly annotate the setting with the @Setting annotation so the documentation can be generated with the autodoc plugin

Copy link

This issue is stale because it has been open for 14 days with no activity.

@github-actions github-actions bot added the stale Open for x days with no activity label Mar 26, 2024
@ndr-brt
Copy link
Member

ndr-brt commented Mar 26, 2024

the documentation is being posted on gh pages, this is the current one:
https://eclipse-edc.github.io/Connector/autodoc/0.5.2-SNAPSHOT/

@github-actions github-actions bot removed the stale Open for x days with no activity label Mar 27, 2024
@segoranov
Copy link
Contributor

documenting the settings in a readme is highly discouraged because they get outdated easily, better to properly annotate the setting with the @Setting annotation so the documentation can be generated with the autodoc plugin

already outdated, apart from the inconsistency mentioned in the issue, there is also no such parameter used at all: edc.transfer.client.selector.strategy (it's in the readme file only).

can i take this issue?

segoranov pushed a commit to segoranov/DataSpaceConnector that referenced this issue Apr 4, 2024
… files (eclipse-edc#3986)

* avoid outdatedness of the documentation of config params in README files
segoranov added a commit to segoranov/DataSpaceConnector that referenced this issue Apr 4, 2024
… files (eclipse-edc#3986)

* avoid outdatedness of the documentation of config params in README files
segoranov added a commit to segoranov/DataSpaceConnector that referenced this issue Apr 4, 2024
… files (eclipse-edc#3986)

* avoid outdatedness of the documentation of config params in README files
ndr-brt pushed a commit that referenced this issue Apr 10, 2024
… files (#4084)

* docs: annotate properly settings and refer to autodoc from the README files (#3986)

* avoid outdatedness of the documentation of config params in README files

* docs: simplify description of setting

Co-authored-by: Jim Marino <jim.marino@gmail.com>

* fix: avoid string to int conversion in config parameters

* docs: annotate properly the settings and unify their usage

* chore: update DEPENDENCIES file

* chore: remove unneeded sections from README

---------

Co-authored-by: Jim Marino <jim.marino@gmail.com>
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 good first issue Good for newcomers
Projects
None yet
3 participants