Skip to content

docs: provide complete TaskAuthorizationProvider example - #1146

Open
hutiefang76 wants to merge 2 commits into
a2aproject:mainfrom
hutiefang76:frank/a2a-java-1143-auth-docs
Open

docs: provide complete TaskAuthorizationProvider example#1146
hutiefang76 wants to merge 2 commits into
a2aproject:mainfrom
hutiefang76:frank/a2a-java-1143-auth-docs

Conversation

@hutiefang76

Copy link
Copy Markdown
Contributor

Description

The authorization examples referenced undefined isOwner and ownershipStore symbols, so they could not be used as a complete implementation of TaskAuthorizationProvider.

This change:

  • provides all required imports and a concrete concurrent ownership map;
  • keeps read and write checks fail-closed;
  • records ownership atomically with putIfAbsent; and
  • keeps the development guide and API Javadoc examples aligned.

Testing

  • mvn -o -Dmaven.repo.local=/tmp/m2-a2a-1143 -f docs/pom.xml -DskipTests package
  • mvn -Dmaven.repo.local=/tmp/m2-a2a-1143 -pl server-common -am -DskipTests package

Fixes #1143 🦕

Comment thread docs/content/dev/authorization.md Outdated

@ApplicationScoped
public class MyTaskAuthorizationProvider implements TaskAuthorizationProvider {
private final ConcurrentMap<String, String> owners = new ConcurrentHashMap<>();

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think the variable ownershipStore makes more sense

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Renamed the map to ownershipStore in both the development guide and the API Javadoc example in ff9a841.

Comment thread docs/content/dev/authorization.md Outdated
@Override
public boolean checkRead(ServerCallContext context, String taskId, TaskOperation op) {
return isOwner(context.getUser(), taskId);
String owner = owners.get(taskId);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think this should be in a private method isOwner

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Extracted the ownership lookup into a private isOwner(context, taskId) helper in both examples in ff9a841.

Comment thread docs/content/dev/authorization.md Outdated
@Override
public boolean checkWrite(ServerCallContext context, String taskId, TaskOperation op) {
return isOwner(context.getUser(), taskId);
return checkRead(context, taskId, op);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

calling isOwner here to avoid calling API code

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Both checkRead and checkWrite now call the private isOwner helper directly in ff9a841. The development guide and Javadoc examples are identical. The docs package and server-common reactor package builds passed locally (tests skipped for this documentation-only change).

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

Successfully merging this pull request may close these issues.

[Bug]: Documentation should provide a valid example of TaskAuthorizationProvider

2 participants