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

IP Groups: Continue to support IP Groups for v4.0 and add better support for specifying ranges such as subnets. #700

Closed
eaquigley opened this issue Jul 9, 2014 · 13 comments
Assignees
Labels
Type: Feature a feature request

Comments

@eaquigley
Copy link
Contributor


Author Name: Kevin Condon (@kcondon)
Original Redmine Issue: 4156, https://redmine.hmdc.harvard.edu/issues/4156
Original Date: 2014-06-25
Original Assignee: Gustavo Durand


Retaining IP Group authentication was mentioned as being a requirement by a current partner.

Note: there are two types of notation we might consider supporting: subnetwork notation and CIDR notation.

This was originally requested here:
#60969: v2.0 - IP Groups: Need to support adding ip addresses by ranges that don't span entire octet.

https://help.hmdc.harvard.edu/Ticket/Display.html?id=60969

I recently needed to add some large ip address ranges that did not
cover the entire octet (number between the decimals).

For example: 199.94.0.-199.94.47. required me to enter 48 individual
addresses.

Then recently here:
#180843: Feature request: support for subnet notation when defining IP groups

https://help.hmdc.harvard.edu/Ticket/Display.html?id=180843

Not sure if this is on the agenda at all, and it's a minor point, but it
would be really useful to be able to define IP groups using subnet
notation rather than just * as a wildcard.

Our use case is, we have some universities with multiple class C
networks in the middle of a range. (e.g. 134.117.10-135). No easy way to
input this at present.

@eaquigley
Copy link
Contributor Author


Original Redmine Comment
Author Name: Philip Durbin (@pdurbin)
Original Date: 2014-06-30T11:51:16Z


Retaining IP Group authentication was mentioned as being a requirement by a current partner.

Other partners subsequently weighed in on the importance of this feature: https://groups.google.com/d/msg/dataverse-community/hQO1UDMa-yY/GlhMVIbYR_8J

@eaquigley eaquigley added this to the Dataverse 4.0: In Review milestone Jul 9, 2014
@scolapasta scolapasta modified the milestones: Beta 3 - Dataverse 4.0, In Review - Dataverse 4.0 Jul 15, 2014
@eaquigley eaquigley modified the milestones: Beta 3 - Dataverse 4.0, Beta 7 - Dataverse 4.0 Aug 19, 2014
@michbarsinai
Copy link
Member

Moving to Beta 9, with the rest of the groups.

@michbarsinai
Copy link
Member

Role assignment via API now supported. Permissions granted to IP groups are given to the user as requested.

Now struggling with the UI crashing in the roles page.

michbarsinai added a commit that referenced this issue Jan 20, 2015
@michbarsinai
Copy link
Member

Ready for QA.
Notes for testing:

  • I'm unsure how permissions granted via groups work with search in the UI, so may be better to test via API initially.
  • For IP groups, one can spoof the IP address by adding the "X-Forwarded-For" header. May be easier than creating and removing groups and assignments.

@michbarsinai michbarsinai removed their assignment Jan 20, 2015
@kcondon
Copy link
Contributor

kcondon commented Jan 21, 2015

Encountered a foreign key constraint but realized I had not run the db updates. Works now. However, there is no key required for adding an ip group. Shouldn't there be?

@pdurbin
Copy link
Member

pdurbin commented Jan 21, 2015

For IP groups, one can spoof the IP address by adding the "X-Forwarded-For" header.

@michbarsinai this is just for testing, right? You'll be taking this out? Is there a ticket for this? Being able to spoof an IP address so easily is alarming.

@kcondon
Copy link
Contributor

kcondon commented Jan 21, 2015

All of the create, list, delete functions work with basic test data. Having trouble assigning a role, have asked Michael for advice.

@michbarsinai
Copy link
Member

@pdurbin indeed. But this is how apache tells Glassfish who sent the request originally. We can do a test, but I assume apache would overwrite the header if someone was spoofing it.
Maybe we should add a Setting for this, so admins can turn the functionality off.

@pdurbin
Copy link
Member

pdurbin commented Jan 21, 2015

I assume apache would overwrite the header

Please keep in mind that we are not 100% sure that we will be fronting Glassfish with Apache we have only ever seen the session bug in #647 when Glassfish is fronted with Apache.

@michbarsinai I'm asking you to think about the scenario where Glassfish is not fronted with Apache and the security implications of letting users specify the "X-Forwarded-For" header.

@michbarsinai
Copy link
Member

very good point. also easy to solve. I'll open an issue.

On Jan 22, 2015, at 01:39, Philip Durbin notifications@github.com wrote:

I assume apache would overwrite the header

Please keep in mind that we are not 100% sure that we will be fronting Glassfish with Apache we have only ever seen the session bug in #647 #647 when Glassfish is fronted with Apache.

@michbarsinai https://github.com/michbarsinai I'm asking you to think about the scenario where Glassfish is not fronted with Apache and the security implications of letting users specify the "X-Forwarded-For" header.


Reply to this email directly or view it on GitHub #700 (comment).

@michbarsinai
Copy link
Member

Created #1368

@kcondon
Copy link
Contributor

kcondon commented Jan 23, 2015

Basic functionality in place, opened tickets for completing integration with rest of app. Closing

@kcondon kcondon closed this as completed Jan 23, 2015
@pdurbin
Copy link
Member

pdurbin commented Oct 3, 2016

See also pull request #3103 which was merged in 4.5.1 to address #1380 and related issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Feature a feature request
Projects
None yet
Development

No branches or pull requests

5 participants