Skip to content

Conversation

@jthiltges
Copy link
Contributor

This allows unmapped GSI clients to access data via the root protocol as an anonymous user.

This allows anonymous users to access data via the root protocol.
@bbockelm
Copy link
Contributor

bbockelm commented Jun 2, 2023

Hm - I'm uncomfortable with this one. It allows mistakes access all of a sudden, right?

@jthiltges jthiltges marked this pull request as draft June 2, 2023 19:35
@jthiltges
Copy link
Contributor Author

Marking as draft to work through an idea that Brian suggested: only treat a client as anonymous if the username lookup failed and there was no mapping done. (gmapopt:trymap resulting in a DN hash, rather than a username.)

… as anonymous

For names generated from DN or DN hash, treat the client as anonymous if
the username cannot be found.

If the name came from a grid-mapfile, fail if the username cannot be
found.
@bbockelm
Copy link
Contributor

bbockelm commented Jun 3, 2023

Since it does allocate a string and do a hash map lookup, can you only do the gridmap mapping check in the failure case where you use it? Maybe peel it off as a separate static method?

@jthiltges
Copy link
Contributor Author

That's a good idea, thanks.

I'd been thinking about turning it around: only try getpwnam() if gridmap.name == "1". Otherwise consider the client as anonymous. It would keep the string allocation and hash lookup, but avoid NSS hits with DNs and DN hashes.

@bbockelm
Copy link
Contributor

bbockelm commented Jun 5, 2023

That'd be even better! Note that you would then need to check that this is a GSI mapping to make sure the behavior doesn't change for other protocols.

@jthiltges jthiltges marked this pull request as ready for review June 5, 2023 17:15
@djw8605
Copy link
Member

djw8605 commented Jun 5, 2023

Is this tested? If so, seems simple enough.

@bbockelm
Copy link
Contributor

bbockelm commented Jun 5, 2023

If this tests out fine, I’m happy with the implementation approach.

@jthiltges
Copy link
Contributor Author

I've applied it to the production Nebraska SEs, and no issues to report. I think it's ready to consider squashing and merging.

@djw8605 djw8605 merged commit 444fef5 into opensciencegrid:master Jun 9, 2023
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.

3 participants