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

UAC issues on Windows #40

Closed
evertiro opened this issue Apr 16, 2014 · 22 comments
Closed

UAC issues on Windows #40

evertiro opened this issue Apr 16, 2014 · 22 comments
Assignees
Milestone

Comments

@evertiro
Copy link

On Windows, hostsupdater fails unless vagrant is run as an administrator. Unfortunately, this is really not the smart way to run vagrant. I know you can elevate privileges through WIN32OLE, but I'm not familiar enough with ruby to actually patch it myself. Thoughts?

@tkdb
Copy link
Contributor

tkdb commented May 8, 2014

Open %SystemRoot%\system32\drivers\etc in Windows Explorer, locate the hosts file, right-click it and set the usage rights to the current user (the one you run vagrant under) to allow file modification.

The plugin has no problems to work then, you don't need to run vagrant as administrator.

tkdb added a commit to tkdb/vagrant-hostsupdater that referenced this issue May 9, 2014
Two windows cmd scripts that can work together to change permissions for
the current user to modify the hosts file.

win-hosts-check.cmd check if the permissions on the file are sufficient.

If not it will call win-hosts-set.cmd which will trigger the windows
permission dialog asking for enough rights to midify the permissions on
the hosts file for the current user.

See issue agiledivider#40
tkdb added a commit to tkdb/vagrant-hostsupdater that referenced this issue May 9, 2014
Two windows cmd scripts that can work together to change permissions for
the current user to modify the hosts file.

win-hosts-check.cmd check if the permissions on the file are sufficient.

If not it will call win-hosts-set.cmd which will trigger the windows
permission dialog asking for enough rights to midify the permissions on
the hosts file for the current user.

See issue agiledivider#40
@tkdb
Copy link
Contributor

tkdb commented May 9, 2014

If you want to do that in a semi-automatic fashion, I've created some cmd scripts that are capable of doing so.

@itsananderson
Copy link

So, I probably should have checked the project issues before working on this, but I've also written a solution.

https://github.com/itsananderson/vagrant-hostsupdater

The main difference between my workaround and @tkdb's is mine asks for UAC escalation "on-demand" each time it modifies the hosts file, so write permissions on the hosts file aren't changed at all.

If we want to use all or part of my solution, I can clean it up for a PR.

@tkdb
Copy link
Contributor

tkdb commented Jun 2, 2014

@itsananderson Thanks for the feedback. From quick review I'd say your code is worth for PR (I'm not speaking for the vagrant-hostsupdater project here just my personal opinion as a user and developer), if you can make it into a PR referencing this issue would be great, then I can give it a test-drive here on my box.

Edit: Later I spotted Powershell as dependency which is not necessarily part of windows - available checks (as well as fail checks) are missing.

willenglishiv pushed a commit to willenglishiv/vagrant-hostsupdater that referenced this issue Nov 27, 2014
Two windows cmd scripts that can work together to change permissions for
the current user to modify the hosts file.

win-hosts-check.cmd check if the permissions on the file are sufficient.

If not it will call win-hosts-set.cmd which will trigger the windows
permission dialog asking for enough rights to midify the permissions on
the hosts file for the current user.

See issue agiledivider#40
@jeremiahsmall
Copy link

@itsananderson +1 for removing powershell dependency and PR'ing this.

@itsananderson
Copy link

I got busy with other things and lost track of this. I think removing the PS dependency should be doable, I just need to find the time to rewrite it. I'll see if I can set aside some time over the next weekend or two. Feel free to ping me if I forget again :)

EDIT: Looks like there was already a commit back in November that seems to solve this issue?

@bretmogilefsky
Copy link

@itsananderson Can you give a pointer to that commit please? Not seeing it in the commit log for this repository.

@itsananderson
Copy link

Oh, I was wrong. I saw this: willenglishiv@d5b031c and incorrectly thought it was in the original repo.

I'll see if I can use that commit as a reference for doing UAC elevation in CMD, so we can get this fix into a PR.

@bretmogilefsky
Copy link

@itsananderson That'd be great!

@polarathene
Copy link

Just ran into this issue, since the last commit to the plugin was 2013 has development on this repo likely stopped for good? Which repo were you going to send the PR to?

@jeremiahsmall
Copy link

Hmm, @Cogitatio has been totally silent on GitHub for the previous year. Maybe he wouldn't mind if someone took over maintenance?

@cgsmith
Copy link
Collaborator

cgsmith commented Nov 4, 2015

Hey @jeremiahsmall I will be maintaining the project for @Cogitatio - I'll be submitting updates shortly.

@jeremiahsmall
Copy link

Okay. Good to see the project lives.

@cgsmith
Copy link
Collaborator

cgsmith commented Nov 4, 2015

👍

@cgsmith
Copy link
Collaborator

cgsmith commented Nov 8, 2015

@itsananderson if youd like to submit a pull request that's be great. Otherwise I'll take a crack at it in a couple weeks.

@e7d
Copy link

e7d commented Nov 9, 2015

I definetly support @itsananderson solution. Do the PR! 👍

I'm currently using the 1.0.1 version from @cgsmith correcting this first start crash. Addressing this issue would make it perfect for Windows users, I guess.

@QWp6t
Copy link

QWp6t commented Nov 15, 2015

@cgsmith @itsananderson

This should hopefully be a good starting point...

         if !File.writable_real?(@@hosts_path)
           sudo(%Q(sh -c 'echo "#{content}" >> #@@hosts_path'))
+        elsif Vagrant::Util::Platform.windows?
+          require 'tmpdir'
+          uuid = @machine.id || @machine.config.hostsupdater.id
+          hashedId = Digest::MD5.hexdigest(uuid)
+          tmpPath = File.join(Dir.tmpdir, 'hosts-' + uuid + '.cmd')
+          File.open(tmpPath, "w") do |tmpFile|
+            entries.each { |line| tmpFile.puts(">>\"#{@@hosts_path}\" echo #{line}") }
+          end
+          sudo(tmpPath)
+          File.delete(tmpPath)
         else
           content = "\n" + content
           hostsFile = File.open(@@hosts_path, "a")
       def sudo(command)
         return if !command
+        if Vagrant::Util::Platform.windows?
+          require 'win32ole'
+          args = command.split(" ")
+          command = args.shift
+          sh = WIN32OLE.new('Shell.Application')
+          sh.ShellExecute(command, args.join(" "), '', 'runas', 0)
+          return
+        end
         `sudo #{command}`
       end

@QWp6t
Copy link

QWp6t commented Nov 16, 2015

So far that code above has been working for me. Has no 3rd party dependencies. Works with multiple entries. I think that's probably the way to go. But feel free to test it on your own.

If we could get sh.ShellExecute to properly send line-breaks to sh.exe, then we could remove the temp file workaround. But so far in my testing, it just puts everything on one line.

@cgsmith cgsmith added this to the 1.0.2 milestone Dec 8, 2015
@cgsmith cgsmith self-assigned this Dec 8, 2015
@oniric85
Copy link

Is there any more work to do for this one? I'm willing to help as this would be a really useful addition!

@cgsmith
Copy link
Collaborator

cgsmith commented Jul 20, 2016

Hi @oniric85 if you are willing you could pull down the code and test it out on your machine! That would be really useful! Feel free to document what it takes you to get the code running on the wiki or by writing some Markdown - thanks for offering to help!

@QWp6t
Copy link

QWp6t commented Jul 20, 2016

I submitted a PR to fix this almost a year ago. I've since moved on to vagrant-hostmanager, though.

@cgsmith
Copy link
Collaborator

cgsmith commented Jul 20, 2016

Thanks @QWp6t I'd like to have it tested on a Windows machine (I dont have one) and will gladly merge it in if some others can verify it is working.

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

No branches or pull requests

10 participants