10
\$\begingroup\$

This script is designed to locate and download the first repository in your sidebar on the GitHub repository page. It downloads the .zip file for that repo and saves it to the default location.

I built this script as a test to determine what it would take to perform this type of operation since I needed to use it on something else, but this script itself is the actual script I use (very) infrequently.

Basically, any/all PowerShell idioms would be awesome to have feedback on. The comments are there for the person I was writing the script as an example for, but again this is the script I'm worried about. I want to continually improve it.

# This method will finish an IE download by forcing the 'ALT+S' key several times to ensure that the "Save" button gets
# hit. It then waits a specified amount of time before returning so that the user can allow the file download to be
# expected to have finished.
Function Finish-Download([__ComObject]$ie, [int]$iterations, [int]$sleep) {
    Write-Verbose "Finish-Download: Entered 'Finish-Download ie iterations'"
    Write-Debug "Finish-Download: 'iterations': $iterations"
    Write-Debug ("Finish-Download: 'ie.HWND': " + $ie.HWND)

    Write-Warning "Please do not make any mouse or keyboard interactions for this next step."

    Write-Verbose "Finish-Download: Locating 'ie' process"
    $ieProc = Get-Process | Where-Object { $_.Name -eq "iexplore" -and $_.MainWindowHandle -eq $ie.HWND }

    Write-Verbose "Finish-Download: Attaching 'WScript.Shell' to 'ieProc'"
    Write-Debug ("Finish-Download: 'ieProc.Id': " + $ieProc.Id)
    $wScript = New-Object -ComObject WScript.Shell
    if (-not $wScript.AppActivate($ieProc.Id)) {
        Write-Error "Failed to attach WScript.Shell to appropriate process."
        Return $false
    }

    # For SOME reason, WScript takes a moment to activate the process. So we need to sleep. During this part it's
    # important that you don't make any mouse/keyboard movements.
    # Another hiccup: IE isn't a great browser, as a result sometimes the ALT+S command doesn't work properly, so
    # we're sending it multiple times to make sure that it get's received by Internet Explorer.
    Sleep 1
    Write-Verbose "Finish-Download: Sending 'ALT' then 'ALT+S' $iterations times"
    do {
        Write-Debug "Finish-Download: 'iterations': $iterations"
        $wScript.SendKeys("%")
        $wScript.SendKeys("%S")
        $iterations = $iterations - 1
    } while ($iterations -gt 0)
    # Note: this will save it to the default location. You can mess with WScript if you wish to use the "Save As"

    Sleep 1
    Write-Warning "You may use your mouse and keyboard again."

    Write-Verbose "Finish-Download: Sleep for $sleep seconds to wait for download to complete"
    Sleep $sleep
}

# You'll have to mess with a lot of this for your specific web-page layout, but it should be fairly self-explanatory
Function Download-File([string]$url, [string]$user, [string]$pass) {
    try {
        Write-Verbose "Download-File: Entered 'Download-File url user pass'"
        Write-Debug "Download-File: 'url': $url"
        Write-Debug "Download-File: 'user': $user"
        Write-Debug ("Download-File: 'pass': " + $pass.Length.ToString() + " chars")
        # This is the ID for the login text-box and the password text-box
        $usernameElement = "login_field"
        $passwordElement = "password"

        Write-Verbose "Download-File: Creating 'Wait-NavigationCompleted[__ComObject]'"
        # Define a function to wait for navigation to finishe
        Function Wait-NavigationCompleted([__ComObject]$ie) {
            Write-Verbose "Wait-NavigationCompleted: Waiting for navigation to complete..."
            do {
                Write-Debug ("Wait-NavigationCompleted: 'ie.Busy': " + $ie.Busy)
                sleep 1
            }
            until (-not ($ie.Busy))
        }

        Write-Verbose "Download-File: Creating 'ie' object"
        # Define our IE object
        $ie = New-Object -ComObject InternetExplorer.Application

        # We NEED IE to be visible to download stuff appropriate, since we can't follow a specific URL to get to it. (If it's
        # possible to do so, let me know and I can work up an example of parsing a URL out of the page to get it to be
        # navigateable.)
        $ie.Visible = $true
        Write-Debug ("Download-File: 'ie.Visible': " + $ie.Visible.ToString())

        Write-Verbose "Download-File: Navigate to '$url'"
        # Navigate to the root login page
        $ie.Navigate($url)

        Wait-NavigationCompleted($ie)

        Write-Verbose "Download-File: Creating 'doc' object"
        # Thanks to how programming works, we only need to set $doc once
        $doc = $ie.Document

        try {
            Write-Verbose "Download-File: Attempting to log in."
            Write-Debug "Download-File: 'usernameElement': $usernameElement"
            Write-Debug "Download-File: 'passwordElement': $passwordElement"
            $doc.getElementById($usernameElement).value = $user
            $doc.getElementById($passwordElement).value = $pass

            # Here you can either use:
            # $doc.getElementById("button-id").click()
            # IF it has a valid ID.
            # In the case of the test website I used it doesn't, so we'll iterate all buttons on the page to find one that
            # looks right
            $doc.getElementsByName("commit") | % {
                if ($_.value -eq "Sign in") {
                    $_.click()
                }
            }

            Wait-NavigationCompleted($ie)
        } catch {
            Write-Information "Login failed, user likely already logged in."
        }

        Write-Verbose "Download-File: Looking for button to click"
        # This next part can get tricky, I'm not sure how EAM is set up, but you *may* be able to do this without using
        # virtual mouse clicks. You would have to find what the element information is for the button you want to click
        # (type, anything unique to it like the name). In my example, going to the GitHub repo is just a simple link:

        # Basically, you need to perform each step in this area. Make sure to `Wait-NavigationCompleted($ie)` between each
        # one to ensure that you don't accidentally try to do something else when a page is loading. To check a check-box,
        # you just have to:
        $doc.getElementsByTagName("a") | % {
            if ($_.innerText -ne $null -and $_.innerText.Contains("StackExchange")) {
                $_.click()
            }
        }

        Wait-NavigationCompleted($ie)

        Write-Verbose "Download-File: Clicking 'Download' button"
        # To download a file:
        $doc.getElementsByTagName("a") | % {
            if ($_.innerText -ne $null -and $_.innerText.Contains("Download ZIP")) {
                $_.click()
            }
        }

        Wait-NavigationCompleted($ie)

        # Once you have hit the "download" button, we'll use WScript to send a few commands to finish that process off.
        # Sleep long enough that the file will download. If it's remote, and the net can be slow, I would consider
        # increasing this value a bit. (The second number is sleep time, the first is iterations.)
        Finish-Download $ie 5 5

        Write-Verbose "Download-File: Clicking 'Sign out' button"
        # You should logout so that you don't leave a hanging session next time you run the script (though I did design
        # it to look for a hanging session and ignore it).
        $doc.getElementsByTagName("button") | % {
            if ($_.innerText -ne $null -and $_.innerText.Contains("Sign out")) {
                $_.click()
            }
        }

        Wait-NavigationCompleted($ie)

        Write-Verbose "Download-File: Killing 'ie' process"
        # Kill the IE process
        $ie.Quit()

        Return $true
    } catch {
        $Error.ForEach({ Write-Error $_ })
        Return $false
    }
}

# Alter these to log what you want, valid values are "Continue" and "SilentlyContinue" for each
$VerbosePreference = "Continue"
$DebugPreference = "Continue"
$ProgressPreferece = "Continue"

Write-Verbose "Root: Script started"

# Leave this one as it is
$InformationPreference = "Continue"

# Define your web service to connect to and the login page, include `http://` or `https://`
$webService = "https://github.com/login"

# Define your username and password
$username = "USERNAME"
$password = "PASSWORD"

Write-Information "Beginning file download"
if (Download-File $webService $username $password) {
    Write-Information "File downloaded successfully."
} else {
    Write-Error "File could not be downloaded."
}

Exit

As always, all feedback appreciated. If it's possible to make this take $username and $password in an input, I would love an example of that. Otherwise, it does everything I want and more.

\$\endgroup\$
2
\$\begingroup\$

I hope to have more for you but a couple of things to get started.

Verbose, Debug, Information

I am very happy to see you using these cmdlets for different levels of information output to the user. Its way better then a bunch of Write-Host statements.

You left a small configuration section for the end user to change the preference variables governing the use of the Write- cmdlets in your script. The better way to handle this is to convert your script so that it accepts the common cmdlet parameters e.g. -Verbose, -WhatIf, etc. Have a look at this small example

[CmdletBinding()]
param()

Write-Information "Good Morning Dave"
Write-Verbose "Start of script"

function Get-IdealSpeed{
    Write-Verbose "Beware the implications of driving this fast"
    return "88mph"
}

Function Get-Swear{
    Write-Debug "Writing something naughty. Hide the children"
    Write-Output "Great Scott!"
}

Write-Output "*shrug scripting shoulders*"
Get-IdealSpeed
Get-Swear

You can now call this script optionally with -Verbose and -Debug without having to change preference variables.

This does not really apply with your script since you exit at the end but be careful about changing these session variables as they might have been set to something different before you set them. Some people set these in their profile scripts. Mostly just to make you aware. So depending on how people execute this it might be wise to get the current state of those variables to reset them when you are done with their use. Supporting the common cmdlet parameters seems a better approach though.

Credentials

Blah blah WHY YOU SAVE PLAIN TEXT CREDENTIALS IN FILE. Ok, now that is out of the way. There are a couple of suggestions here.

  1. Prompt the end user for credentials

    You can use the Get-Credential cmdlet to prompt the running user for a username and masked password. This is still technically stored in memory and accessible but better then just leaving it in plain text.

    $userSuppliedCredentials = Get-Credential -Message "Type in your github credentials please" "Username is : $($userSuppliedCredentials.UserName)" "Password is : $($userSuppliedCredentials.GetNetworkCredential().Password)"

  2. Store the credentials, as a securestring, in a file

    You can store secure strings in a file for repeated use. They are designed to be decrypted by the user and computer that created them. Prompting a user for this information might be annoying but arguably more secure.

Nesting function calls

Obviously this is allowed since your code runs but you should at least move function declarations to the top of their respective scopes. That is common practice so it will make them easier to find. I personally would not nest declarations and would declare them in the same scope however the is nothing wrong with that approach in general.

IE automation

It usually is a source of heartache and people would be best to avoid it. There are some cases where you just can't get around it.

I do not have enough experience with this but could you not just use the GitHub API for the better portion of this? I know that you can easily query for repos and I would think you should be able to download the one you need using the release API documentation? I made a quick GitHub account and queried for the one repo (empty) I had. If you had multiple it should be easy to isolate one. Where to go from here is beyond my knowledge as of this writing though.

$gitHubAPIURL = "https://api.github.com"
Invoke-RestMethod -Method Get -Uri "$gitHubAPIURL/user/repos?access_token=my_api_key" | Select -First 1
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.