Skip to content

WIP - Convert accumulo-proxy to single user#58

Closed
DomGarguilo wants to merge 9 commits into
apache:mainfrom
DomGarguilo:singleUser
Closed

WIP - Convert accumulo-proxy to single user#58
DomGarguilo wants to merge 9 commits into
apache:mainfrom
DomGarguilo:singleUser

Conversation

@DomGarguilo

Copy link
Copy Markdown
Member

Fixes #16

@DomGarguilo DomGarguilo self-assigned this Dec 9, 2022
@DomGarguilo
DomGarguilo requested a review from ctubbsii December 12, 2022 17:18
@DomGarguilo
DomGarguilo marked this pull request as ready for review December 14, 2022 15:41
@DomGarguilo

Copy link
Copy Markdown
Member Author

I think there may still be work to be done here but I am going to mark this as ready for review now to start to get some feedback to make sure I am on the right track with these changes.

@ctubbsii ctubbsii left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

For my comments below, I use the term "preferred language client" to refer to the user's code written in Python, Ruby, or whatever, that talks to the proxy service. This is to distinguish it from the proxy service itself, which is an Accumulo client. The preferred language client can also be thought of as the "proxy client", because it is the client to the proxy service. I hope the terminology isn't too confusing.

)

void addSplits (
1:binary login

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think you're probably going to need to keep the login field, but use it differently. It is currently used to track authenticated sessions. I think we still need that, so the preferred language client can authenticate to the proxy with some kind of shared secret. The task at hand is to eliminate the need for the preferred language client to instruct the proxy on how to authenticate to Accumulo, because the proxy should be able to authenticate to Accumulo using the client properties file. But, we still want the proxy to not just allow anybody to connect to it and use it. So, the preferred language client should have some way to authenticate to the proxy. For that, this thrift field can be reused to pass that secret shared between the preferred language client and the proxy.

client = Accumulo.newClient().from(props).build();

try {
String tokenProp = props.getProperty("tokenClass", PasswordToken.class.getName());

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't think you need this check anymore. Accumulo.newClient() should be able to set up the client using whatever token type it supports. That restriction was in place before because the preferred language client could only support passwords... but now that we're not logging in using the preferred language client, and the proxy is setting up the client on its own, we don't need to restrict it.

}

@Override
public void setTableProperty(ByteBuffer login, String tableName, String property, String value)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The shared secret can just be a String. We don't need it to be binary anymore.

@DomGarguilo DomGarguilo mentioned this pull request Dec 22, 2022
@DomGarguilo

Copy link
Copy Markdown
Member Author

Superseded by #59

@DomGarguilo
DomGarguilo deleted the singleUser branch January 18, 2023 16:49
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.

Make the proxy single user

2 participants