Skip to content
This repository has been archived by the owner on Oct 20, 2023. It is now read-only.

auth-bypassing-ip-list #50

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

divinity76
Copy link
Contributor

list of IPs bypassing authentication entirely..

There is something messed up with windows 10 explorer.exe authentication ( see #47 ), this patch allows mounting from windows 10 with some security (essentially a ip whitelist)

haven't implemented any tests for it though.

list of IPs bypassing authentication entirely..

There is something messed up with windows 10 explorer.exe authentication ( see micromata#47 ), this patch allows mounting from windows 10 with some security (essentially a ip whitelist)

haven't implemented any tests for it though.
@chclaus
Copy link
Collaborator

chclaus commented Aug 12, 2023

Hej,
quick update: I'll take care of it next week. 🤞

@chclaus chclaus self-requested a review August 20, 2023 07:39
@chclaus chclaus self-assigned this Aug 20, 2023
Copy link
Collaborator

@chclaus chclaus left a comment

Choose a reason for hiding this comment

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

Hej, thank a lot!

Your code looks good to me. But I think we need a unit test for all ways of authentication. It's a critical part of the application. Do you have the possibility to add a test? I think you could extend the args struct in TestAuthenticate with the requestIpAddress.

Then you could add further szenarios like

{
	"without credentials and incorrect ip address",
	args{
		config:           &Config{Authentication_Bypass_IP_Addresses: []string{"127.0.0.1"}},
		requestIpAddress: "192.168.0.1",
	},
	&AuthInfo{Username: "", Authenticated: false},
	true,
},

@samhocevar do you have any further ideas?

Greets and thanks again for the contribution. 👋 😃

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants